[PATCH] Bluetooth: Fix use-after-free read in hci_cmd_timeout()

[PATCH] Bluetooth: Fix use-after-free read in hci_cmd_timeout()

[Shigeru Yoshida]

syzbot reported use-after-free in hci_cmd_timeout() [1].  The scenario
for the issue is as follows:

Task                                    Workqueue
----------------------------------------------------------------------
hci_dev_open_sync
  ...
  hci_dev_init_sync  <- failed
  ...
  if (hdev->sent_cmd)
    kfree_skb(hdev->sent_cmd)
                                        hci_cmd_timeout
                                          ...
                                          if (hdev->sent_cmd)
                                            sent = hdev->sent_cmd->data
                                                   ^^ UAF occurred
    hdev->sent_cmd = NULL

When hci_dev_init_sync() failed, hci_dev_open_sync() frees
hdev->send_cmd and set it to NULL.  However, hci_cmd_timeout() can run
just after freeing hdev->sent_cmd because hdev->cmd_timer is not
canceled.

This patch fixes the issue by canceling hdev->cmd_timer before freeing
hdev->sent_cmd.

Link: https://syzkaller.appspot.com/bug?id=cb23ebfc8f304f510fb717cb783fe8b496c7ffb1 [1]
Reported-by: syzbot+19a9f729f05272857487@syzkaller.appspotmail.com
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
 net/bluetooth/hci_sync.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 76c3107c9f91..a011065220e4 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -4696,6 +4696,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
 			hdev->flush(hdev);
 
 		if (hdev->sent_cmd) {
+			cancel_delayed_work_sync(&hdev->cmd_timer);
 			kfree_skb(hdev->sent_cmd);
 			hdev->sent_cmd = NULL;
 		}
-- 
2.38.1

RE: Bluetooth: Fix use-after-free read in hci_cmd_timeout()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----
error: patch failed: net/bluetooth/hci_sync.c:4696
error: net/bluetooth/hci_sync.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch


Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: Fix use-after-free read in hci_cmd_timeout()

[Luiz Augusto von Dentz]

Hi Shigeru,

On Sat, Nov 5, 2022 at 12:04 PM Shigeru Yoshida <syoshida@redhat.com> wrote:
>
> syzbot reported use-after-free in hci_cmd_timeout() [1].  The scenario
> for the issue is as follows:
>
> Task                                    Workqueue
> ----------------------------------------------------------------------
> hci_dev_open_sync
>   ...
>   hci_dev_init_sync  <- failed
>   ...
>   if (hdev->sent_cmd)
>     kfree_skb(hdev->sent_cmd)
>                                         hci_cmd_timeout
>                                           ...
>                                           if (hdev->sent_cmd)
>                                             sent = hdev->sent_cmd->data
>                                                    ^^ UAF occurred
>     hdev->sent_cmd = NULL
>
> When hci_dev_init_sync() failed, hci_dev_open_sync() frees
> hdev->send_cmd and set it to NULL.  However, hci_cmd_timeout() can run
> just after freeing hdev->sent_cmd because hdev->cmd_timer is not
> canceled.
>
> This patch fixes the issue by canceling hdev->cmd_timer before freeing
> hdev->sent_cmd.
>
> Link: https://syzkaller.appspot.com/bug?id=cb23ebfc8f304f510fb717cb783fe8b496c7ffb1 [1]
> Reported-by: syzbot+19a9f729f05272857487@syzkaller.appspotmail.com
> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> ---
>  net/bluetooth/hci_sync.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 76c3107c9f91..a011065220e4 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -4696,6 +4696,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
>                         hdev->flush(hdev);
>
>                 if (hdev->sent_cmd) {
> +                       cancel_delayed_work_sync(&hdev->cmd_timer);
>                         kfree_skb(hdev->sent_cmd);
>                         hdev->sent_cmd = NULL;
>                 }
> --
> 2.38.1

A similar fix has already been applied:

https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=64b5c4c8e79c131fe8f135bab5e5dfaa245c5776


-- 
Luiz Augusto von Dentz