diff mbox series

Bluetooth: hci: fix null-ptr-deref in hci_read_supported_codecs

Message ID 20240703113936.228226-1-iam@sung-woo.kim
State New
Headers show
Series Bluetooth: hci: fix null-ptr-deref in hci_read_supported_codecs | expand

Commit Message

Sungwoo Kim July 3, 2024, 11:39 a.m. UTC
KASAN: null-ptr-deref in range [0x0000000000000070-0x0000000000000077]
CPU: 1 PID: 2000 Comm: kworker/u9:5 Not tainted 6.9.0-ga6bcb805883c-dirty #10
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Workqueue: hci7 hci_power_on
RIP: 0010:hci_read_supported_codecs+0xb9/0x870 net/bluetooth/hci_codec.c:138
Code: 08 48 89 ef e8 b8 c1 8f fd 48 8b 75 00 e9 96 00 00 00 49 89 c6 48 ba 00 00 00 00 00 fc ff df 4c 8d 60 70 4c 89 e3 48 c1 eb 03 <0f> b6 04 13 84 c0 0f 85 82 06 00 00 41 83 3c 24 02 77 0a e8 bf 78
RSP: 0018:ffff888120bafac8 EFLAGS: 00010212
RAX: 0000000000000000 RBX: 000000000000000e RCX: ffff8881173f0040
RDX: dffffc0000000000 RSI: ffffffffa58496c0 RDI: ffff88810b9ad1e4
RBP: ffff88810b9ac000 R08: ffffffffa77882a7 R09: 1ffffffff4ef1054
R10: dffffc0000000000 R11: fffffbfff4ef1055 R12: 0000000000000070
R13: 0000000000000000 R14: 0000000000000000 R15: ffff88810b9ac000
FS:  0000000000000000(0000) GS:ffff8881f6c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f6ddaa3439e CR3: 0000000139764003 CR4: 0000000000770ef0
PKRU: 55555554
Call Trace:
 <TASK>
 hci_read_local_codecs_sync net/bluetooth/hci_sync.c:4546 [inline]
 hci_init_stage_sync net/bluetooth/hci_sync.c:3441 [inline]
 hci_init4_sync net/bluetooth/hci_sync.c:4706 [inline]
 hci_init_sync net/bluetooth/hci_sync.c:4742 [inline]
 hci_dev_init_sync net/bluetooth/hci_sync.c:4912 [inline]
 hci_dev_open_sync+0x19a9/0x2d30 net/bluetooth/hci_sync.c:4994
 hci_dev_do_open net/bluetooth/hci_core.c:483 [inline]
 hci_power_on+0x11e/0x560 net/bluetooth/hci_core.c:1015
 process_one_work kernel/workqueue.c:3267 [inline]
 process_scheduled_works+0x8ef/0x14f0 kernel/workqueue.c:3348
 worker_thread+0x91f/0xe50 kernel/workqueue.c:3429
 kthread+0x2cb/0x360 kernel/kthread.c:388
 ret_from_fork+0x4d/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244

Fixes: 8961987f3f5f ("Bluetooth: Enumerate local supported codec and cache details")
Fixes: 9ae664028a9e ("Bluetooth: Add support for Read Local Supported Codecs V2")

Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
 net/bluetooth/hci_codec.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

bluez.test.bot@gmail.com July 3, 2024, 12:43 p.m. UTC | #1
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=867987

---Test result---

Test Summary:
CheckPatch                    FAIL      1.02 seconds
GitLint                       FAIL      0.51 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      30.00 seconds
CheckAllWarning               PASS      32.46 seconds
CheckSparse                   WARNING   38.20 seconds
CheckSmatch                   WARNING   103.78 seconds
BuildKernel32                 PASS      28.89 seconds
TestRunnerSetup               PASS      528.55 seconds
TestRunner_l2cap-tester       PASS      20.29 seconds
TestRunner_iso-tester         PASS      33.14 seconds
TestRunner_bnep-tester        PASS      4.89 seconds
TestRunner_mgmt-tester        FAIL      115.18 seconds
TestRunner_rfcomm-tester      PASS      7.48 seconds
TestRunner_sco-tester         PASS      15.06 seconds
TestRunner_ioctl-tester       PASS      7.98 seconds
TestRunner_mesh-tester        PASS      5.93 seconds
TestRunner_smp-tester         PASS      7.00 seconds
TestRunner_userchan-tester    PASS      5.16 seconds
IncrementalBuild              PASS      27.69 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: hci: fix null-ptr-deref in hci_read_supported_codecs
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#82: 
CPU: 1 PID: 2000 Comm: kworker/u9:5 Not tainted 6.9.0-ga6bcb805883c-dirty #10

total: 0 errors, 1 warnings, 0 checks, 25 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13722112.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: hci: fix null-ptr-deref in hci_read_supported_codecs

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
8: B1 Line exceeds max length (199>80): "Code: 08 48 89 ef e8 b8 c1 8f fd 48 8b 75 00 e9 96 00 00 00 49 89 c6 48 ba 00 00 00 00 00 fc ff df 4c 8d 60 70 4c 89 e3 48 c1 eb 03 <0f> b6 04 13 84 c0 0f 85 82 06 00 00 41 83 3c 24 02 77 0a e8 bf 78"
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_codec.c: note: in included file:./include/net/bluetooth/hci_core.h:147:35: warning: array of flexible structures
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_codec.c: note: in included file:./include/net/bluetooth/hci_core.h:147:35: warning: array of flexible structures
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2

Failed Test Cases
LL Privacy - Add Device 6 (RL is full)               Failed       0.191 seconds


---
Regards,
Linux Bluetooth
Sungwoo Kim July 3, 2024, 9:35 p.m. UTC | #2
Hi Luiz,

On Wed, Jul 3, 2024 at 10:25 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi,
>
> On Wed, Jul 3, 2024 at 7:40 AM Sungwoo Kim <iam@sung-woo.kim> wrote:
> >
> > KASAN: null-ptr-deref in range [0x0000000000000070-0x0000000000000077]
> > CPU: 1 PID: 2000 Comm: kworker/u9:5 Not tainted 6.9.0-ga6bcb805883c-dirty #10
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > Workqueue: hci7 hci_power_on
> > RIP: 0010:hci_read_supported_codecs+0xb9/0x870 net/bluetooth/hci_codec.c:138
> > Code: 08 48 89 ef e8 b8 c1 8f fd 48 8b 75 00 e9 96 00 00 00 49 89 c6 48 ba 00 00 00 00 00 fc ff df 4c 8d 60 70 4c 89 e3 48 c1 eb 03 <0f> b6 04 13 84 c0 0f 85 82 06 00 00 41 83 3c 24 02 77 0a e8 bf 78
> > RSP: 0018:ffff888120bafac8 EFLAGS: 00010212
> > RAX: 0000000000000000 RBX: 000000000000000e RCX: ffff8881173f0040
> > RDX: dffffc0000000000 RSI: ffffffffa58496c0 RDI: ffff88810b9ad1e4
> > RBP: ffff88810b9ac000 R08: ffffffffa77882a7 R09: 1ffffffff4ef1054
> > R10: dffffc0000000000 R11: fffffbfff4ef1055 R12: 0000000000000070
> > R13: 0000000000000000 R14: 0000000000000000 R15: ffff88810b9ac000
> > FS:  0000000000000000(0000) GS:ffff8881f6c00000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f6ddaa3439e CR3: 0000000139764003 CR4: 0000000000770ef0
> > PKRU: 55555554
> > Call Trace:
> >  <TASK>
> >  hci_read_local_codecs_sync net/bluetooth/hci_sync.c:4546 [inline]
> >  hci_init_stage_sync net/bluetooth/hci_sync.c:3441 [inline]
> >  hci_init4_sync net/bluetooth/hci_sync.c:4706 [inline]
> >  hci_init_sync net/bluetooth/hci_sync.c:4742 [inline]
> >  hci_dev_init_sync net/bluetooth/hci_sync.c:4912 [inline]
> >  hci_dev_open_sync+0x19a9/0x2d30 net/bluetooth/hci_sync.c:4994
> >  hci_dev_do_open net/bluetooth/hci_core.c:483 [inline]
> >  hci_power_on+0x11e/0x560 net/bluetooth/hci_core.c:1015
> >  process_one_work kernel/workqueue.c:3267 [inline]
> >  process_scheduled_works+0x8ef/0x14f0 kernel/workqueue.c:3348
> >  worker_thread+0x91f/0xe50 kernel/workqueue.c:3429
> >  kthread+0x2cb/0x360 kernel/kthread.c:388
> >  ret_from_fork+0x4d/0x80 arch/x86/kernel/process.c:147
> >  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
> >
> > Fixes: 8961987f3f5f ("Bluetooth: Enumerate local supported codec and cache details")
> > Fixes: 9ae664028a9e ("Bluetooth: Add support for Read Local Supported Codecs V2")
> >
> > Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> > ---
> >  net/bluetooth/hci_codec.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/net/bluetooth/hci_codec.c b/net/bluetooth/hci_codec.c
> > index 3cc135bb1..5c98eec2c 100644
> > --- a/net/bluetooth/hci_codec.c
> > +++ b/net/bluetooth/hci_codec.c
> > @@ -74,6 +74,9 @@ static void hci_read_codec_capabilities(struct hci_dev *hdev, __u8 transport,
> >
> >                         skb = __hci_cmd_sync_sk(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> >                                                 sizeof(*cmd), cmd, 0, HCI_CMD_TIMEOUT, NULL);
> > +
> > +                       if (!skb)
> > +                               skb = ERR_PTR(-EINVAL);
> >                         if (IS_ERR(skb)) {
> >                                 bt_dev_err(hdev, "Failed to read codec capabilities (%ld)",
> >                                            PTR_ERR(skb));
> > @@ -129,6 +132,8 @@ void hci_read_supported_codecs(struct hci_dev *hdev)
> >         skb = __hci_cmd_sync_sk(hdev, HCI_OP_READ_LOCAL_CODECS, 0, NULL,
> >                                 0, HCI_CMD_TIMEOUT, NULL);
> >
> > +       if (!skb)
> > +               skb = ERR_PTR(-EINVAL);
> >         if (IS_ERR(skb)) {
> >                 bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
> >                            PTR_ERR(skb));
> > @@ -198,6 +203,8 @@ void hci_read_supported_codecs_v2(struct hci_dev *hdev)
> >         skb = __hci_cmd_sync_sk(hdev, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL,
> >                                 0, HCI_CMD_TIMEOUT, NULL);
> >
> > +       if (!skb)
> > +               skb = ERR_PTR(-EINVAL);
> >         if (IS_ERR(skb)) {
> >                 bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
> >                            PTR_ERR(skb));
> > --
> > 2.34.1
>
>
> There is something that doesn't quite add here, we don't return NULL
> on __hci_cmd_sync_sk, if there is an error it will always return

The skb can be NULL if a command returns a status event as there are
no parameters.

> ERR_PTR or perhaps this is the result of rsp_skb being NULL in which
> case we shall check it and actually return ERR_PTR directly from

If returning NULL is not intended, we should also remove a NULL
checking in __hci_cmd_sync_status_sk().

We can do this:
https://gist.github.com/swkim101/5346b4ce5d2b1d4be95ef8dbdcbd9820

> __hci_cmd_sync_sk but Id like to understand under what circumstances
> does rsp_skb can be NULL.

>
>
> --
> Luiz Augusto von Dentz
>

Thanks,
Sungwoo.
diff mbox series

Patch

diff --git a/net/bluetooth/hci_codec.c b/net/bluetooth/hci_codec.c
index 3cc135bb1..5c98eec2c 100644
--- a/net/bluetooth/hci_codec.c
+++ b/net/bluetooth/hci_codec.c
@@ -74,6 +74,9 @@  static void hci_read_codec_capabilities(struct hci_dev *hdev, __u8 transport,
 
 			skb = __hci_cmd_sync_sk(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
 						sizeof(*cmd), cmd, 0, HCI_CMD_TIMEOUT, NULL);
+
+			if (!skb)
+				skb = ERR_PTR(-EINVAL);
 			if (IS_ERR(skb)) {
 				bt_dev_err(hdev, "Failed to read codec capabilities (%ld)",
 					   PTR_ERR(skb));
@@ -129,6 +132,8 @@  void hci_read_supported_codecs(struct hci_dev *hdev)
 	skb = __hci_cmd_sync_sk(hdev, HCI_OP_READ_LOCAL_CODECS, 0, NULL,
 				0, HCI_CMD_TIMEOUT, NULL);
 
+	if (!skb)
+		skb = ERR_PTR(-EINVAL);
 	if (IS_ERR(skb)) {
 		bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
 			   PTR_ERR(skb));
@@ -198,6 +203,8 @@  void hci_read_supported_codecs_v2(struct hci_dev *hdev)
 	skb = __hci_cmd_sync_sk(hdev, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL,
 				0, HCI_CMD_TIMEOUT, NULL);
 
+	if (!skb)
+		skb = ERR_PTR(-EINVAL);
 	if (IS_ERR(skb)) {
 		bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
 			   PTR_ERR(skb));