[PATCH] Bluetooth: hci_sync: Cancel cmd_timer before draining workqueue

[PATCH] Bluetooth: hci_sync: Cancel cmd_timer before draining workqueue

[rabindra789]

cmd_timer can expire while hdev->workqueue is being drained,
causing hci_cmd_timeout to be queued on a draining workqueue.

Cancel cmd_timer before draining the workqueue.

Reported-by: syzbot+48fc5b35b350240fd465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=48fc5b35b350240fd465
Fixes: 40b87657200c ("Bluetooth: hci_core: Rework hci_dev_do_reset() to use hci_sync functions")

Signed-off-by: rabindra789 <rabindrameher116@gmail.com>
---
 net/bluetooth/hci_sync.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index c01c8b58d9e8..e1c28bffdc15 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5545,6 +5545,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)
 	 */
 	hci_dev_set_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE);
 	synchronize_rcu();
+	/* Explicitly cancel works in case scheduled after setting the flag. */
+	cancel_delayed_work_sync(&hdev->cmd_timer);
 
 	if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
 		disable_delayed_work(&hdev->power_off);
-- 
2.43.0

RE: Bluetooth: hci_sync: Cancel cmd_timer before draining workqueue

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2226 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/series/1178060/

---Test result---

Test Summary:
CheckPatch                    PASS      0.77 seconds
VerifyFixes                   PASS      0.14 seconds
VerifySignedoff               PASS      0.14 seconds
GitLint                       PASS      0.35 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      29.13 seconds
CheckAllWarning               PASS      33.26 seconds
CheckSparse                   PASS      35.57 seconds
BuildKernel32                 PASS      29.02 seconds
CheckKernelLLVM               PASS      32.88 seconds
TestRunnerSetup               PASS      780.92 seconds
TestRunner_l2cap-tester       PASS      12.76 seconds
TestRunner_iso-tester         PASS      18.75 seconds
TestRunner_bnep-tester        PASS      3.13 seconds
TestRunner_mgmt-tester        FAIL      61.09 seconds
TestRunner_rfcomm-tester      PASS      4.48 seconds
TestRunner_sco-tester         PASS      5.55 seconds
TestRunner_ioctl-tester       PASS      4.63 seconds
TestRunner_mesh-tester        FAIL      7.82 seconds
TestRunner_smp-tester         PASS      4.33 seconds
TestRunner_userchan-tester    PASS      3.19 seconds
TestRunner_6lowpan-tester     PASS      4.40 seconds
IncrementalBuild              PASS      26.94 seconds

Details
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 503, Passed: 502 (99.8%), Failed: 1, Not Run: 0

Failed Test Cases
Pairing Acceptor - LE Security Level Changed         Failed       0.026 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.418 seconds
Mesh - Send cancel - 2                               Timed out    2.000 seconds


https://github.com/bluez/bluetooth-next/pull/850

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: hci_sync: Cancel cmd_timer before draining workqueue

[Luiz Augusto von Dentz]

Hi Rabindra,

On Fri, Oct 2, 2026 at 12:48 AM rabindra789 <rabindrameher116@gmail.com> wrote:
>
> cmd_timer can expire while hdev->workqueue is being drained,
> causing hci_cmd_timeout to be queued on a draining workqueue.
>
> Cancel cmd_timer before draining the workqueue.
>
> Reported-by: syzbot+48fc5b35b350240fd465@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=48fc5b35b350240fd465
> Fixes: 40b87657200c ("Bluetooth: hci_core: Rework hci_dev_do_reset() to use hci_sync functions")
>
> Signed-off-by: rabindra789 <rabindrameher116@gmail.com>
> ---
>  net/bluetooth/hci_sync.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index c01c8b58d9e8..e1c28bffdc15 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5545,6 +5545,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)
>          */
>         hci_dev_set_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE);
>         synchronize_rcu();
> +       /* Explicitly cancel works in case scheduled after setting the flag. */
> +       cancel_delayed_work_sync(&hdev->cmd_timer);

I'm not following here. If a command is outstanding and times out
while we are draining the queue that should be flagged. Or perhaps the
problem is that hci_cmd_sync_cancel_sync don't actually behave like
hci_cmd_sync_cancel which clears the cmd_timer whenever it cancels the
cmd_sync work:

static void hci_cmd_sync_cancel_work(struct work_struct *work)
{
        struct hci_dev *hdev = container_of(work, struct hci_dev,
cmd_sync_cancel_work);

        cancel_delayed_work_sync(&hdev->cmd_timer);
        cancel_delayed_work_sync(&hdev->ncmd_timer);
        atomic_set(&hdev->cmd_cnt, 1);

        wake_up_interruptible(&hdev->req_wait_q);
}

>         if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
>                 disable_delayed_work(&hdev->power_off);
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_sync: Cancel cmd_timer before draining workqueue

[Luiz Augusto von Dentz]

Hi,

On Fri, Oct 2, 2026 at 10:29 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Rabindra,
>
> On Fri, Oct 2, 2026 at 12:48 AM rabindra789 <rabindrameher116@gmail.com> wrote:
> >
> > cmd_timer can expire while hdev->workqueue is being drained,
> > causing hci_cmd_timeout to be queued on a draining workqueue.
> >
> > Cancel cmd_timer before draining the workqueue.
> >
> > Reported-by: syzbot+48fc5b35b350240fd465@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=48fc5b35b350240fd465
> > Fixes: 40b87657200c ("Bluetooth: hci_core: Rework hci_dev_do_reset() to use hci_sync functions")
> >
> > Signed-off-by: rabindra789 <rabindrameher116@gmail.com>
> > ---
> >  net/bluetooth/hci_sync.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index c01c8b58d9e8..e1c28bffdc15 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -5545,6 +5545,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> >          */
> >         hci_dev_set_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE);
> >         synchronize_rcu();
> > +       /* Explicitly cancel works in case scheduled after setting the flag. */
> > +       cancel_delayed_work_sync(&hdev->cmd_timer);
>
> I'm not following here. If a command is outstanding and times out
> while we are draining the queue that should be flagged. Or perhaps the
> problem is that hci_cmd_sync_cancel_sync don't actually behave like
> hci_cmd_sync_cancel which clears the cmd_timer whenever it cancels the
> cmd_sync work:
>
> static void hci_cmd_sync_cancel_work(struct work_struct *work)
> {
>         struct hci_dev *hdev = container_of(work, struct hci_dev,
> cmd_sync_cancel_work);
>
>         cancel_delayed_work_sync(&hdev->cmd_timer);
>         cancel_delayed_work_sync(&hdev->ncmd_timer);
>         atomic_set(&hdev->cmd_cnt, 1);
>
>         wake_up_interruptible(&hdev->req_wait_q);
> }

Maybe something like the following:

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 1fe11d3ef1aa..87b9f96af057 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -702,6 +702,12 @@ void hci_cmd_sync_cancel_sync(struct hci_dev
*hdev, int err)
                hdev->req_result = err < 0 ? -err : err;
                WRITE_ONCE(hdev->req_status, HCI_REQ_CANCELED);

+               if (!hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
+                       cancel_delayed_work_sync(&hdev->cmd_timer);
+                       cancel_delayed_work_sync(&hdev->ncmd_timer);
+                       atomic_set(&hdev->cmd_cnt, 1);
+               }
+
                wake_up_interruptible(&hdev->req_wait_q);
        }
 }

> >         if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
> >                 disable_delayed_work(&hdev->power_off);
> > --
> > 2.43.0
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_sync: Cancel cmd_timer before draining workqueue

[Rabindra Kumar Meher]

Hi Luiz,

Thanks, I checked the reported path again. And the warning is from a
cmd_timer already armed before hci_dev_close_sync(). The re-arm path
is already blocked by HCI_CMD_DRAIN_WORKQUEUE + synchronize_rcu().
I think cmd_timer still needs to be canceled before drain_workqueue().
Moving cancel_delayed_work_sync() into hci_cmd_sync_cancel_sync()
would make hci_cmd_timeout() synchronouslly cancel its own cmd_timer.
Also, in the reported path req_status has already been reset to 0
before hci_dev_close_sync() calls hci_cmd_sync_cancel_sync(), so the
HCI_REQ_PEND casewould not cover this warning.

Thanks,
Rabindra


On Sat, Oct 3, 2026 at 4:52 PM Rabindra Kumar Meher
<rabindrameher116@gmail.com> wrote:
>
> Hi Luiz,
>
> Thanks, I checked the reported path again. And the warning is from a cmd_timer already armed before hci_dev_close_sync(). The re-arm path is already blocked by HCI_CMD_DRAIN_WORKQUEUE + synchronize_rcu().
> I think cmd_timer still needs to be canceled before drain_workqueue(). Moving cancel_delayed_work_sync() into hci_cmd_sync_cancel_sync() would make hci_cmd_timeout() synchronousllly cancel its own cmd_timer. Also, in the reported path req_status has already been reset to 0 before hci_dev_close_sync() calls hci_cmd_sync_cancel_sync() so the HCI_REQ_PEND case would not cover this warning.
>
> Thanks,
> Rabindra
>
> On Fri, Oct 2, 2026 at 8:02 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi,
>>
>> On Fri, Oct 2, 2026 at 10:29 AM Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>> >
>> > Hi Rabindra,
>> >
>> > On Fri, Oct 2, 2026 at 12:48 AM rabindra789 <rabindrameher116@gmail.com> wrote:
>> > >
>> > > cmd_timer can expire while hdev->workqueue is being drained,
>> > > causing hci_cmd_timeout to be queued on a draining workqueue.
>> > >
>> > > Cancel cmd_timer before draining the workqueue.
>> > >
>> > > Reported-by: syzbot+48fc5b35b350240fd465@syzkaller.appspotmail.com
>> > > Closes: https://syzkaller.appspot.com/bug?extid=48fc5b35b350240fd465
>> > > Fixes: 40b87657200c ("Bluetooth: hci_core: Rework hci_dev_do_reset() to use hci_sync functions")
>> > >
>> > > Signed-off-by: rabindra789 <rabindrameher116@gmail.com>
>> > > ---
>> > >  net/bluetooth/hci_sync.c | 2 ++
>> > >  1 file changed, 2 insertions(+)
>> > >
>> > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
>> > > index c01c8b58d9e8..e1c28bffdc15 100644
>> > > --- a/net/bluetooth/hci_sync.c
>> > > +++ b/net/bluetooth/hci_sync.c
>> > > @@ -5545,6 +5545,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)
>> > >          */
>> > >         hci_dev_set_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE);
>> > >         synchronize_rcu();
>> > > +       /* Explicitly cancel works in case scheduled after setting the flag. */
>> > > +       cancel_delayed_work_sync(&hdev->cmd_timer);
>> >
>> > I'm not following here. If a command is outstanding and times out
>> > while we are draining the queue that should be flagged. Or perhaps the
>> > problem is that hci_cmd_sync_cancel_sync don't actually behave like
>> > hci_cmd_sync_cancel which clears the cmd_timer whenever it cancels the
>> > cmd_sync work:
>> >
>> > static void hci_cmd_sync_cancel_work(struct work_struct *work)
>> > {
>> >         struct hci_dev *hdev = container_of(work, struct hci_dev,
>> > cmd_sync_cancel_work);
>> >
>> >         cancel_delayed_work_sync(&hdev->cmd_timer);
>> >         cancel_delayed_work_sync(&hdev->ncmd_timer);
>> >         atomic_set(&hdev->cmd_cnt, 1);
>> >
>> >         wake_up_interruptible(&hdev->req_wait_q);
>> > }
>>
>> Maybe something like the following:
>>
>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
>> index 1fe11d3ef1aa..87b9f96af057 100644
>> --- a/net/bluetooth/hci_sync.c
>> +++ b/net/bluetooth/hci_sync.c
>> @@ -702,6 +702,12 @@ void hci_cmd_sync_cancel_sync(struct hci_dev
>> *hdev, int err)
>>                 hdev->req_result = err < 0 ? -err : err;
>>                 WRITE_ONCE(hdev->req_status, HCI_REQ_CANCELED);
>>
>> +               if (!hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
>> +                       cancel_delayed_work_sync(&hdev->cmd_timer);
>> +                       cancel_delayed_work_sync(&hdev->ncmd_timer);
>> +                       atomic_set(&hdev->cmd_cnt, 1);
>> +               }
>> +
>>                 wake_up_interruptible(&hdev->req_wait_q);
>>         }
>>  }
>>
>> > >         if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
>> > >                 disable_delayed_work(&hdev->power_off);
>> > > --
>> > > 2.43.0
>> > >
>> >
>> >
>> > --
>> > Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
>
> --
> Rabindra Kumar Meher



-- 
Rabindra Kumar Meher