From: Vasyl Vavrychuk <[email protected]>
`cancel_work_sync(&hdev->power_on)` was moved to hci_dev_close_sync in
commit [1] to ensure that power_on work is canceled after HCI interface
down.
But, in certain cases power_on work function may call hci_dev_close_sync
itself: hci_power_on -> hci_dev_do_close -> hci_dev_close_sync ->
cancel_work_sync(&hdev->power_on), causing deadlock. In particular, this
happens when device is rfkilled on boot. To avoid deadlock, move
power_on work canceling out of hci_dev_do_close/hci_dev_close_sync.
Deadlock introduced by commit [1] was reported in [2,3] as broken
suspend. Suspend did not work because `hdev->req_lock` held as result of
`power_on` work deadlock. In fact, other BT features were not working.
It was not observed when testing [1] since it was verified without
rfkill in place.
NOTE: It is not needed to cancel power_on work from other places where
hci_dev_do_close/hci_dev_close_sync is called in case:
* Requests were serialized due to `hdev->req_workqueue`. The power_on
work is first in that workqueue.
* hci_rfkill_set_block which won't close device anyway until HCI_SETUP
is on.
* hci_sock_release which runs after hci_sock_bind which ensures
HCI_SETUP was cleared.
As result, behaviour is the same as in pre-dd06ed7 commit, except
power_on work cancel added to hci_dev_close.
[1]: commit dd06ed7ad057 ("Bluetooth: core: Fix missing power_on work cancel on HCI close")
[2]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/
Fixes: ff7f2926114d ("Bluetooth: core: Fix missing power_on work cancel on HCI close")
Signed-off-by: Vasyl Vavrychuk <[email protected]>
Reported-by: Max Krummenacher <[email protected]>
Reported-by: Mateusz Jonczyk <[email protected]>
Tested-by: Max Krummenacher <[email protected]>
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/hci_core.c | 3 +++
net/bluetooth/hci_sync.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 05c13f639b94..27e90eb4bf4c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -571,6 +571,7 @@ int hci_dev_close(__u16 dev)
goto done;
}
+ cancel_work_sync(&hdev->power_on);
if (hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF))
cancel_delayed_work(&hdev->power_off);
@@ -2677,6 +2678,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
list_del(&hdev->list);
write_unlock(&hci_dev_list_lock);
+ cancel_work_sync(&hdev->power_on);
+
hci_cmd_sync_clear(hdev);
hci_unregister_suspend_notifier(hdev);
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 489c420b477c..dcaeb0c18292 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -4139,7 +4139,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
bt_dev_dbg(hdev, "");
- cancel_work_sync(&hdev->power_on);
cancel_delayed_work(&hdev->power_off);
cancel_delayed_work(&hdev->ncmd_timer);
--
2.35.3
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=656752
---Test result---
Test Summary:
CheckPatch FAIL 1.57 seconds
GitLint FAIL 0.78 seconds
SubjectPrefix PASS 0.80 seconds
BuildKernel PASS 31.82 seconds
BuildKernel32 PASS 27.22 seconds
Incremental Build with patchesPASS 40.95 seconds
TestRunner: Setup PASS 466.79 seconds
TestRunner: l2cap-tester PASS 16.42 seconds
TestRunner: bnep-tester PASS 5.59 seconds
TestRunner: mgmt-tester PASS 95.09 seconds
TestRunner: rfcomm-tester PASS 9.00 seconds
TestRunner: sco-tester PASS 8.75 seconds
TestRunner: smp-tester PASS 8.95 seconds
TestRunner: userchan-tester PASS 5.74 seconds
Details
##############################
Test: CheckPatch - FAIL - 1.57 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: core: Fix deadlock on hci_power_on_sync.\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#108:
[1]: commit dd06ed7ad057 ("Bluetooth: core: Fix missing power_on work cancel on HCI close")
ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit fatal: unsaf ("ace/src' is owned by someone else)")'
#108:
[1]: commit dd06ed7ad057 ("Bluetooth: core: Fix missing power_on work cancel on HCI close")
total: 1 errors, 1 warnings, 0 checks, 22 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/12906901.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL - 0.78 seconds
Run gitlint with rule in .gitlint
Bluetooth: core: Fix deadlock on hci_power_on_sync.
1: T3 Title has trailing punctuation (.): "Bluetooth: core: Fix deadlock on hci_power_on_sync."
33: B1 Line exceeds max length (91>80): "[1]: commit dd06ed7ad057 ("Bluetooth: core: Fix missing power_on work cancel on HCI close")"
---
Regards,
Linux Bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 5 Jul 2022 10:25:01 -0700 you wrote:
> From: Vasyl Vavrychuk <[email protected]>
>
> `cancel_work_sync(&hdev->power_on)` was moved to hci_dev_close_sync in
> commit [1] to ensure that power_on work is canceled after HCI interface
> down.
>
> But, in certain cases power_on work function may call hci_dev_close_sync
> itself: hci_power_on -> hci_dev_do_close -> hci_dev_close_sync ->
> cancel_work_sync(&hdev->power_on), causing deadlock. In particular, this
> happens when device is rfkilled on boot. To avoid deadlock, move
> power_on work canceling out of hci_dev_do_close/hci_dev_close_sync.
>
> [...]
Here is the summary with links:
- Bluetooth: core: Fix deadlock on hci_power_on_sync.
https://git.kernel.org/bluetooth/bluetooth-next/c/bf909caec390
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html