diff mbox series

[RESEND] Bluetooth: Fix deadlock in vhci_send_frame

Message ID 20231110014605.2068231-1-yinghsu@chromium.org (mailing list archive)
State Accepted
Commit 0be46f8900b032f37cc77420f54775ff42ca4f14
Headers show
Series [RESEND] Bluetooth: Fix deadlock in vhci_send_frame | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/TestRunner_l2cap-tester success TestRunner PASS
tedd_an/TestRunner_iso-tester success TestRunner PASS
tedd_an/TestRunner_bnep-tester success TestRunner PASS
tedd_an/TestRunner_mgmt-tester success TestRunner PASS
tedd_an/TestRunner_rfcomm-tester success TestRunner PASS
tedd_an/TestRunner_sco-tester success TestRunner PASS
tedd_an/TestRunner_ioctl-tester success TestRunner PASS
tedd_an/TestRunner_mesh-tester success TestRunner PASS
tedd_an/TestRunner_smp-tester success TestRunner PASS
tedd_an/TestRunner_userchan-tester success TestRunner PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Ying Hsu Nov. 10, 2023, 1:46 a.m. UTC 
syzbot found a potential circular dependency leading to a deadlock:
    -> #3 (&hdev->req_lock){+.+.}-{3:3}:
    __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
    __mutex_lock kernel/locking/mutex.c:732 [inline]
    mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
    hci_dev_do_close+0x3f/0x9f net/bluetooth/hci_core.c:551
    hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
    rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
    rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
    vfs_write+0x277/0xcf5 fs/read_write.c:594
    ksys_write+0x19b/0x2bd fs/read_write.c:650
    do_syscall_x64 arch/x86/entry/common.c:55 [inline]
    do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
    entry_SYSCALL_64_after_hwframe+0x61/0xcb

    -> #2 (rfkill_global_mutex){+.+.}-{3:3}:
    __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
    __mutex_lock kernel/locking/mutex.c:732 [inline]
    mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
    rfkill_register+0x30/0x7e3 net/rfkill/core.c:1045
    hci_register_dev+0x48f/0x96d net/bluetooth/hci_core.c:2622
    __vhci_create_device drivers/bluetooth/hci_vhci.c:341 [inline]
    vhci_create_device+0x3ad/0x68f drivers/bluetooth/hci_vhci.c:374
    vhci_get_user drivers/bluetooth/hci_vhci.c:431 [inline]
    vhci_write+0x37b/0x429 drivers/bluetooth/hci_vhci.c:511
    call_write_iter include/linux/fs.h:2109 [inline]
    new_sync_write fs/read_write.c:509 [inline]
    vfs_write+0xaa8/0xcf5 fs/read_write.c:596
    ksys_write+0x19b/0x2bd fs/read_write.c:650
    do_syscall_x64 arch/x86/entry/common.c:55 [inline]
    do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
    entry_SYSCALL_64_after_hwframe+0x61/0xcb

    -> #1 (&data->open_mutex){+.+.}-{3:3}:
    __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
    __mutex_lock kernel/locking/mutex.c:732 [inline]
    mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
    vhci_send_frame+0x68/0x9c drivers/bluetooth/hci_vhci.c:75
    hci_send_frame+0x1cc/0x2ff net/bluetooth/hci_core.c:2989
    hci_sched_acl_pkt net/bluetooth/hci_core.c:3498 [inline]
    hci_sched_acl net/bluetooth/hci_core.c:3583 [inline]
    hci_tx_work+0xb94/0x1a60 net/bluetooth/hci_core.c:3654
    process_one_work+0x901/0xfb8 kernel/workqueue.c:2310
    worker_thread+0xa67/0x1003 kernel/workqueue.c:2457
    kthread+0x36a/0x430 kernel/kthread.c:319
    ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298

    -> #0 ((work_completion)(&hdev->tx_work)){+.+.}-{0:0}:
    check_prev_add kernel/locking/lockdep.c:3053 [inline]
    check_prevs_add kernel/locking/lockdep.c:3172 [inline]
    validate_chain kernel/locking/lockdep.c:3787 [inline]
    __lock_acquire+0x2d32/0x77fa kernel/locking/lockdep.c:5011
    lock_acquire+0x273/0x4d5 kernel/locking/lockdep.c:5622
    __flush_work+0xee/0x19f kernel/workqueue.c:3090
    hci_dev_close_sync+0x32f/0x1113 net/bluetooth/hci_sync.c:4352
    hci_dev_do_close+0x47/0x9f net/bluetooth/hci_core.c:553
    hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
    rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
    rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
    vfs_write+0x277/0xcf5 fs/read_write.c:594
    ksys_write+0x19b/0x2bd fs/read_write.c:650
    do_syscall_x64 arch/x86/entry/common.c:55 [inline]
    do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
    entry_SYSCALL_64_after_hwframe+0x61/0xcb

This change removes the need for acquiring the open_mutex in
vhci_send_frame, thus eliminating the potential deadlock while
maintaining the required packet ordering.

Fixes: 92d4abd66f70 ("Bluetooth: vhci: Fix race when opening vhci device")
Signed-off-by: Ying Hsu <yinghsu@chromium.org>
---
Tested this commit using a C reproducer on qemu-x86_64.

 drivers/bluetooth/hci_vhci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

bluez.test.bot@gmail.com Nov. 10, 2023, 2:09 a.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=800102

---Test result---

Test Summary:
CheckPatch                    PASS      1.72 seconds
GitLint                       PASS      0.48 seconds
SubjectPrefix                 PASS      0.29 seconds
BuildKernel                   PASS      26.98 seconds
CheckAllWarning               PASS      29.80 seconds
CheckSparse                   PASS      34.96 seconds
CheckSmatch                   PASS      97.68 seconds
BuildKernel32                 PASS      26.29 seconds
TestRunnerSetup               PASS      410.70 seconds
TestRunner_l2cap-tester       PASS      22.77 seconds
TestRunner_iso-tester         PASS      41.80 seconds
TestRunner_bnep-tester        PASS      6.88 seconds
TestRunner_mgmt-tester        PASS      169.80 seconds
TestRunner_rfcomm-tester      PASS      10.89 seconds
TestRunner_sco-tester         PASS      14.49 seconds
TestRunner_ioctl-tester       PASS      12.42 seconds
TestRunner_mesh-tester        PASS      8.79 seconds
TestRunner_smp-tester         PASS      10.53 seconds
TestRunner_userchan-tester    PASS      7.24 seconds
IncrementalBuild              PASS      25.39 seconds



---
Regards,
Linux Bluetooth
Ying Hsu Nov. 13, 2023, 3:52 a.m. UTC | #2 
Hi Luiz and Arkadiusz,

I appreciate the effort you put into ensuring the quality of the
kernel, if it looks like the review might need more time, could we
consider reverting commit 92d4abd66f70 ("Bluetooth: vhci: Fix race
when opening vhci device") in the interim? This would help maintain
stability until the new patch is approved.

Best regards,
Ying


On Fri, Nov 10, 2023 at 9:46 AM Ying Hsu <yinghsu@chromium.org> wrote:
>
> syzbot found a potential circular dependency leading to a deadlock:
>     -> #3 (&hdev->req_lock){+.+.}-{3:3}:
>     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
>     __mutex_lock kernel/locking/mutex.c:732 [inline]
>     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
>     hci_dev_do_close+0x3f/0x9f net/bluetooth/hci_core.c:551
>     hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
>     rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
>     rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
>     vfs_write+0x277/0xcf5 fs/read_write.c:594
>     ksys_write+0x19b/0x2bd fs/read_write.c:650
>     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
>     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
>     entry_SYSCALL_64_after_hwframe+0x61/0xcb
>
>     -> #2 (rfkill_global_mutex){+.+.}-{3:3}:
>     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
>     __mutex_lock kernel/locking/mutex.c:732 [inline]
>     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
>     rfkill_register+0x30/0x7e3 net/rfkill/core.c:1045
>     hci_register_dev+0x48f/0x96d net/bluetooth/hci_core.c:2622
>     __vhci_create_device drivers/bluetooth/hci_vhci.c:341 [inline]
>     vhci_create_device+0x3ad/0x68f drivers/bluetooth/hci_vhci.c:374
>     vhci_get_user drivers/bluetooth/hci_vhci.c:431 [inline]
>     vhci_write+0x37b/0x429 drivers/bluetooth/hci_vhci.c:511
>     call_write_iter include/linux/fs.h:2109 [inline]
>     new_sync_write fs/read_write.c:509 [inline]
>     vfs_write+0xaa8/0xcf5 fs/read_write.c:596
>     ksys_write+0x19b/0x2bd fs/read_write.c:650
>     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
>     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
>     entry_SYSCALL_64_after_hwframe+0x61/0xcb
>
>     -> #1 (&data->open_mutex){+.+.}-{3:3}:
>     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
>     __mutex_lock kernel/locking/mutex.c:732 [inline]
>     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
>     vhci_send_frame+0x68/0x9c drivers/bluetooth/hci_vhci.c:75
>     hci_send_frame+0x1cc/0x2ff net/bluetooth/hci_core.c:2989
>     hci_sched_acl_pkt net/bluetooth/hci_core.c:3498 [inline]
>     hci_sched_acl net/bluetooth/hci_core.c:3583 [inline]
>     hci_tx_work+0xb94/0x1a60 net/bluetooth/hci_core.c:3654
>     process_one_work+0x901/0xfb8 kernel/workqueue.c:2310
>     worker_thread+0xa67/0x1003 kernel/workqueue.c:2457
>     kthread+0x36a/0x430 kernel/kthread.c:319
>     ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298
>
>     -> #0 ((work_completion)(&hdev->tx_work)){+.+.}-{0:0}:
>     check_prev_add kernel/locking/lockdep.c:3053 [inline]
>     check_prevs_add kernel/locking/lockdep.c:3172 [inline]
>     validate_chain kernel/locking/lockdep.c:3787 [inline]
>     __lock_acquire+0x2d32/0x77fa kernel/locking/lockdep.c:5011
>     lock_acquire+0x273/0x4d5 kernel/locking/lockdep.c:5622
>     __flush_work+0xee/0x19f kernel/workqueue.c:3090
>     hci_dev_close_sync+0x32f/0x1113 net/bluetooth/hci_sync.c:4352
>     hci_dev_do_close+0x47/0x9f net/bluetooth/hci_core.c:553
>     hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
>     rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
>     rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
>     vfs_write+0x277/0xcf5 fs/read_write.c:594
>     ksys_write+0x19b/0x2bd fs/read_write.c:650
>     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
>     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
>     entry_SYSCALL_64_after_hwframe+0x61/0xcb
>
> This change removes the need for acquiring the open_mutex in
> vhci_send_frame, thus eliminating the potential deadlock while
> maintaining the required packet ordering.
>
> Fixes: 92d4abd66f70 ("Bluetooth: vhci: Fix race when opening vhci device")
> Signed-off-by: Ying Hsu <yinghsu@chromium.org>
> ---
> Tested this commit using a C reproducer on qemu-x86_64.
>
>  drivers/bluetooth/hci_vhci.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> index f3892e9ce800..572d68d52965 100644
> --- a/drivers/bluetooth/hci_vhci.c
> +++ b/drivers/bluetooth/hci_vhci.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <asm/unaligned.h>
>
> +#include <linux/atomic.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> @@ -44,6 +45,7 @@ struct vhci_data {
>         bool wakeup;
>         __u16 msft_opcode;
>         bool aosp_capable;
> +       atomic_t initialized;
>  };
>
>  static int vhci_open_dev(struct hci_dev *hdev)
> @@ -75,11 +77,10 @@ static int vhci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>
>         memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
>
> -       mutex_lock(&data->open_mutex);
>         skb_queue_tail(&data->readq, skb);
> -       mutex_unlock(&data->open_mutex);
>
> -       wake_up_interruptible(&data->read_wait);
> +       if (atomic_read(&data->initialized))
> +               wake_up_interruptible(&data->read_wait);
>         return 0;
>  }
>
> @@ -464,7 +465,8 @@ static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
>         skb_put_u8(skb, 0xff);
>         skb_put_u8(skb, opcode);
>         put_unaligned_le16(hdev->id, skb_put(skb, 2));
> -       skb_queue_tail(&data->readq, skb);
> +       skb_queue_head(&data->readq, skb);
> +       atomic_inc(&data->initialized);
>
>         wake_up_interruptible(&data->read_wait);
>         return 0;
> --
> 2.43.0.rc0.421.g78406f8d94-goog
>
Luiz Augusto von Dentz Nov. 13, 2023, 4:13 a.m. UTC | #3 
Hi Ying,

On Sun, Nov 12, 2023 at 10:52 PM Ying Hsu <yinghsu@chromium.org> wrote:
>
> Hi Luiz and Arkadiusz,
>
> I appreciate the effort you put into ensuring the quality of the
> kernel, if it looks like the review might need more time, could we
> consider reverting commit 92d4abd66f70 ("Bluetooth: vhci: Fix race
> when opening vhci device") in the interim? This would help maintain
> stability until the new patch is approved.

Sorry for not responding before, I'm on vacation, I will try to
prioritize this one as soon as I'm back.

> Best regards,
> Ying
>
>
> On Fri, Nov 10, 2023 at 9:46 AM Ying Hsu <yinghsu@chromium.org> wrote:
> >
> > syzbot found a potential circular dependency leading to a deadlock:
> >     -> #3 (&hdev->req_lock){+.+.}-{3:3}:
> >     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
> >     __mutex_lock kernel/locking/mutex.c:732 [inline]
> >     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
> >     hci_dev_do_close+0x3f/0x9f net/bluetooth/hci_core.c:551
> >     hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
> >     rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
> >     rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
> >     vfs_write+0x277/0xcf5 fs/read_write.c:594
> >     ksys_write+0x19b/0x2bd fs/read_write.c:650
> >     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
> >     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
> >     entry_SYSCALL_64_after_hwframe+0x61/0xcb
> >
> >     -> #2 (rfkill_global_mutex){+.+.}-{3:3}:
> >     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
> >     __mutex_lock kernel/locking/mutex.c:732 [inline]
> >     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
> >     rfkill_register+0x30/0x7e3 net/rfkill/core.c:1045
> >     hci_register_dev+0x48f/0x96d net/bluetooth/hci_core.c:2622
> >     __vhci_create_device drivers/bluetooth/hci_vhci.c:341 [inline]
> >     vhci_create_device+0x3ad/0x68f drivers/bluetooth/hci_vhci.c:374
> >     vhci_get_user drivers/bluetooth/hci_vhci.c:431 [inline]
> >     vhci_write+0x37b/0x429 drivers/bluetooth/hci_vhci.c:511
> >     call_write_iter include/linux/fs.h:2109 [inline]
> >     new_sync_write fs/read_write.c:509 [inline]
> >     vfs_write+0xaa8/0xcf5 fs/read_write.c:596
> >     ksys_write+0x19b/0x2bd fs/read_write.c:650
> >     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
> >     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
> >     entry_SYSCALL_64_after_hwframe+0x61/0xcb
> >
> >     -> #1 (&data->open_mutex){+.+.}-{3:3}:
> >     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
> >     __mutex_lock kernel/locking/mutex.c:732 [inline]
> >     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
> >     vhci_send_frame+0x68/0x9c drivers/bluetooth/hci_vhci.c:75
> >     hci_send_frame+0x1cc/0x2ff net/bluetooth/hci_core.c:2989
> >     hci_sched_acl_pkt net/bluetooth/hci_core.c:3498 [inline]
> >     hci_sched_acl net/bluetooth/hci_core.c:3583 [inline]
> >     hci_tx_work+0xb94/0x1a60 net/bluetooth/hci_core.c:3654
> >     process_one_work+0x901/0xfb8 kernel/workqueue.c:2310
> >     worker_thread+0xa67/0x1003 kernel/workqueue.c:2457
> >     kthread+0x36a/0x430 kernel/kthread.c:319
> >     ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298
> >
> >     -> #0 ((work_completion)(&hdev->tx_work)){+.+.}-{0:0}:
> >     check_prev_add kernel/locking/lockdep.c:3053 [inline]
> >     check_prevs_add kernel/locking/lockdep.c:3172 [inline]
> >     validate_chain kernel/locking/lockdep.c:3787 [inline]
> >     __lock_acquire+0x2d32/0x77fa kernel/locking/lockdep.c:5011
> >     lock_acquire+0x273/0x4d5 kernel/locking/lockdep.c:5622
> >     __flush_work+0xee/0x19f kernel/workqueue.c:3090
> >     hci_dev_close_sync+0x32f/0x1113 net/bluetooth/hci_sync.c:4352
> >     hci_dev_do_close+0x47/0x9f net/bluetooth/hci_core.c:553
> >     hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
> >     rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
> >     rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
> >     vfs_write+0x277/0xcf5 fs/read_write.c:594
> >     ksys_write+0x19b/0x2bd fs/read_write.c:650
> >     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
> >     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
> >     entry_SYSCALL_64_after_hwframe+0x61/0xcb
> >
> > This change removes the need for acquiring the open_mutex in
> > vhci_send_frame, thus eliminating the potential deadlock while
> > maintaining the required packet ordering.
> >
> > Fixes: 92d4abd66f70 ("Bluetooth: vhci: Fix race when opening vhci device")
> > Signed-off-by: Ying Hsu <yinghsu@chromium.org>
> > ---
> > Tested this commit using a C reproducer on qemu-x86_64.
> >
> >  drivers/bluetooth/hci_vhci.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> > index f3892e9ce800..572d68d52965 100644
> > --- a/drivers/bluetooth/hci_vhci.c
> > +++ b/drivers/bluetooth/hci_vhci.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/module.h>
> >  #include <asm/unaligned.h>
> >
> > +#include <linux/atomic.h>
> >  #include <linux/kernel.h>
> >  #include <linux/init.h>
> >  #include <linux/slab.h>
> > @@ -44,6 +45,7 @@ struct vhci_data {
> >         bool wakeup;
> >         __u16 msft_opcode;
> >         bool aosp_capable;
> > +       atomic_t initialized;
> >  };
> >
> >  static int vhci_open_dev(struct hci_dev *hdev)
> > @@ -75,11 +77,10 @@ static int vhci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> >
> >         memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
> >
> > -       mutex_lock(&data->open_mutex);
> >         skb_queue_tail(&data->readq, skb);
> > -       mutex_unlock(&data->open_mutex);
> >
> > -       wake_up_interruptible(&data->read_wait);
> > +       if (atomic_read(&data->initialized))
> > +               wake_up_interruptible(&data->read_wait);
> >         return 0;
> >  }
> >
> > @@ -464,7 +465,8 @@ static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
> >         skb_put_u8(skb, 0xff);
> >         skb_put_u8(skb, opcode);
> >         put_unaligned_le16(hdev->id, skb_put(skb, 2));
> > -       skb_queue_tail(&data->readq, skb);
> > +       skb_queue_head(&data->readq, skb);
> > +       atomic_inc(&data->initialized);
> >
> >         wake_up_interruptible(&data->read_wait);
> >         return 0;
> > --
> > 2.43.0.rc0.421.g78406f8d94-goog
> >
patchwork-bot+bluetooth@kernel.org Nov. 17, 2023, 4:20 p.m. UTC | #4 
Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 10 Nov 2023 01:46:05 +0000 you wrote:
> syzbot found a potential circular dependency leading to a deadlock:
>     -> #3 (&hdev->req_lock){+.+.}-{3:3}:
>     __mutex_lock_common+0x1b6/0x1bc2 kernel/locking/mutex.c:599
>     __mutex_lock kernel/locking/mutex.c:732 [inline]
>     mutex_lock_nested+0x17/0x1c kernel/locking/mutex.c:784
>     hci_dev_do_close+0x3f/0x9f net/bluetooth/hci_core.c:551
>     hci_rfkill_set_block+0x130/0x1ac net/bluetooth/hci_core.c:935
>     rfkill_set_block+0x1e6/0x3b8 net/rfkill/core.c:345
>     rfkill_fop_write+0x2d8/0x672 net/rfkill/core.c:1274
>     vfs_write+0x277/0xcf5 fs/read_write.c:594
>     ksys_write+0x19b/0x2bd fs/read_write.c:650
>     do_syscall_x64 arch/x86/entry/common.c:55 [inline]
>     do_syscall_64+0x51/0xba arch/x86/entry/common.c:93
>     entry_SYSCALL_64_after_hwframe+0x61/0xcb
> 
> [...]

Here is the summary with links:
  - [RESEND] Bluetooth: Fix deadlock in vhci_send_frame
    https://git.kernel.org/bluetooth/bluetooth-next/c/0be46f8900b0

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index f3892e9ce800..572d68d52965 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -11,6 +11,7 @@ 
 #include <linux/module.h>
 #include <asm/unaligned.h>
 
+#include <linux/atomic.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -44,6 +45,7 @@  struct vhci_data {
 	bool wakeup;
 	__u16 msft_opcode;
 	bool aosp_capable;
+	atomic_t initialized;
 };
 
 static int vhci_open_dev(struct hci_dev *hdev)
@@ -75,11 +77,10 @@  static int vhci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 
 	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
 
-	mutex_lock(&data->open_mutex);
 	skb_queue_tail(&data->readq, skb);
-	mutex_unlock(&data->open_mutex);
 
-	wake_up_interruptible(&data->read_wait);
+	if (atomic_read(&data->initialized))
+		wake_up_interruptible(&data->read_wait);
 	return 0;
 }
 
@@ -464,7 +465,8 @@  static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
 	skb_put_u8(skb, 0xff);
 	skb_put_u8(skb, opcode);
 	put_unaligned_le16(hdev->id, skb_put(skb, 2));
-	skb_queue_tail(&data->readq, skb);
+	skb_queue_head(&data->readq, skb);
+	atomic_inc(&data->initialized);
 
 	wake_up_interruptible(&data->read_wait);
 	return 0;