2024-06-10 11:25:17

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH] Bluetooth: hci_core: cancel rx_work,cmd_work,tx_work,power_on,error_reset works upon hci_unregister_dev()

syzbot is reporting that calling hci_release_dev() from hci_error_reset()
due to hci_dev_put() from hci_error_reset() can cause deadlock at
destroy_workqueue(), for hci_error_reset() is called from
hdev->req_workqueue which destroy_workqueue() needs to flush.

We need to make sure that hdev->{rx_work,cmd_work,tx_work} which are
queued into hdev->workqueue and hdev->{power_on,error_reset} which are
queued into hdev->req_workqueue are no longer running by the moment

destroy_workqueue(hdev->workqueue);
destroy_workqueue(hdev->req_workqueue);

are called from hci_release_dev().

Call cancel_work_sync() on these work items from hci_unregister_dev()
as soon as hdev->list is removed from hci_dev_list.

Reported-by: syzbot <[email protected]>
Closes: https://syzkaller.appspot.com/bug?extid=da0a9c9721e36db712e8
Signed-off-by: Tetsuo Handa <[email protected]>
---
Completely untested. Please do tests with lockdep enabled before committing.
Maybe it is too early to cancel hdev->{rx_work,cmd_work,tx_work}.
Maybe there are more work items which should be canceled before
hci_unregister_dev() completes. I don't know...

net/bluetooth/hci_core.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dd3b0f501018..dbbe5e2da210 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2751,7 +2751,11 @@ void hci_unregister_dev(struct hci_dev *hdev)
list_del(&hdev->list);
write_unlock(&hci_dev_list_lock);

+ cancel_work_sync(&hdev->rx_work);
+ cancel_work_sync(&hdev->cmd_work);
+ cancel_work_sync(&hdev->tx_work);
cancel_work_sync(&hdev->power_on);
+ cancel_work_sync(&hdev->error_reset);

hci_cmd_sync_clear(hdev);

--
2.18.4




2024-06-10 11:51:04

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: hci_core: cancel rx_work,cmd_work,tx_work,power_on,error_reset works upon hci_unregister_dev()

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

---Test result---

Test Summary:
CheckPatch PASS 0.68 seconds
GitLint FAIL 0.50 seconds
SubjectPrefix PASS 0.13 seconds
BuildKernel PASS 29.41 seconds
CheckAllWarning PASS 32.19 seconds
CheckSparse PASS 37.62 seconds
CheckSmatch FAIL 35.74 seconds
BuildKernel32 PASS 29.74 seconds
TestRunnerSetup PASS 514.54 seconds
TestRunner_l2cap-tester PASS 20.03 seconds
TestRunner_iso-tester PASS 28.23 seconds
TestRunner_bnep-tester PASS 4.74 seconds
TestRunner_mgmt-tester PASS 111.15 seconds
TestRunner_rfcomm-tester PASS 7.28 seconds
TestRunner_sco-tester PASS 49.37 seconds
TestRunner_ioctl-tester PASS 7.67 seconds
TestRunner_mesh-tester PASS 7.84 seconds
TestRunner_smp-tester PASS 6.87 seconds
TestRunner_userchan-tester PASS 4.97 seconds
IncrementalBuild PASS 27.45 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: hci_core: cancel rx_work,cmd_work,tx_work,power_on,error_reset works upon hci_unregister_dev()

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
1: T1 Title exceeds max length (105>80): "Bluetooth: hci_core: cancel rx_work,cmd_work,tx_work,power_on,error_reset works upon hci_unregister_dev()"
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o'
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2


---
Regards,
Linux Bluetooth

2024-06-14 15:20:53

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_core: cancel rx_work,cmd_work,tx_work,power_on,error_reset works upon hci_unregister_dev()

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Mon, 10 Jun 2024 20:00:32 +0900 you wrote:
> syzbot is reporting that calling hci_release_dev() from hci_error_reset()
> due to hci_dev_put() from hci_error_reset() can cause deadlock at
> destroy_workqueue(), for hci_error_reset() is called from
> hdev->req_workqueue which destroy_workqueue() needs to flush.
>
> We need to make sure that hdev->{rx_work,cmd_work,tx_work} which are
> queued into hdev->workqueue and hdev->{power_on,error_reset} which are
> queued into hdev->req_workqueue are no longer running by the moment
>
> [...]

Here is the summary with links:
- Bluetooth: hci_core: cancel rx_work,cmd_work,tx_work,power_on,error_reset works upon hci_unregister_dev()
https://git.kernel.org/bluetooth/bluetooth-next/c/5b41aa213455

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html