public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame
@ 2023-11-10  1:46 Ying Hsu
  2023-11-10  2:09 ` [RESEND] " bluez.test.bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ying Hsu @ 2023-11-10  1:46 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: luiz.dentz, linux-kernel, chromeos-bluetooth-upstreaming,
	arkadiusz.bokowy, johan.hedberg, marcel, Ying Hsu

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [RESEND] Bluetooth: Fix deadlock in vhci_send_frame
  2023-11-10  1:46 [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame Ying Hsu
@ 2023-11-10  2:09 ` bluez.test.bot
  2023-11-13  3:52 ` [PATCH RESEND] " Ying Hsu
  2023-11-17 16:20 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2023-11-10  2:09 UTC (permalink / raw)
  To: linux-bluetooth, yinghsu

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

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame
  2023-11-10  1:46 [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame Ying Hsu
  2023-11-10  2:09 ` [RESEND] " bluez.test.bot
@ 2023-11-13  3:52 ` Ying Hsu
  2023-11-13  4:13   ` Luiz Augusto von Dentz
  2023-11-17 16:20 ` patchwork-bot+bluetooth
  2 siblings, 1 reply; 5+ messages in thread
From: Ying Hsu @ 2023-11-13  3:52 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz, arkadiusz.bokowy
  Cc: linux-kernel, chromeos-bluetooth-upstreaming, johan.hedberg,
	marcel

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame
  2023-11-13  3:52 ` [PATCH RESEND] " Ying Hsu
@ 2023-11-13  4:13   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-11-13  4:13 UTC (permalink / raw)
  To: Ying Hsu
  Cc: linux-bluetooth, arkadiusz.bokowy, linux-kernel,
	chromeos-bluetooth-upstreaming, johan.hedberg, marcel

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
> >



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame
  2023-11-10  1:46 [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame Ying Hsu
  2023-11-10  2:09 ` [RESEND] " bluez.test.bot
  2023-11-13  3:52 ` [PATCH RESEND] " Ying Hsu
@ 2023-11-17 16:20 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+bluetooth @ 2023-11-17 16:20 UTC (permalink / raw)
  To: Ying Hsu
  Cc: linux-bluetooth, luiz.dentz, linux-kernel,
	chromeos-bluetooth-upstreaming, arkadiusz.bokowy, johan.hedberg,
	marcel

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-11-17 16:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-10  1:46 [PATCH RESEND] Bluetooth: Fix deadlock in vhci_send_frame Ying Hsu
2023-11-10  2:09 ` [RESEND] " bluez.test.bot
2023-11-13  3:52 ` [PATCH RESEND] " Ying Hsu
2023-11-13  4:13   ` Luiz Augusto von Dentz
2023-11-17 16:20 ` patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox