2022-09-28 20:20:20

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [PATCH v3] Bluetooth: Prevent double register of suspend

From: Abhishek Pandit-Subedi <[email protected]>

Suspend notifier should only be registered and unregistered once per
hdev. Simplify this by only registering during driver registration and
simply exiting early when HCI_USER_CHANNEL is set.

Reported-by: syzbot <[email protected]>
Fixes: 359ee4f834f5 (Bluetooth: Unregister suspend with userchannel)
Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
---
This is fixing a syzbot reported warning. Tested in the following ways:
* Normal start-up of driver with bluez.
* Start/stop loop using HCI_USER_CHANNEL (sock path).
* USB reset triggering hci_dev_unregister (driver path).

------------[ cut here ]------------
double register detected
WARNING: CPU: 0 PID: 2657 at kernel/notifier.c:27
notifier_chain_register kernel/notifier.c:27 [inline]
WARNING: CPU: 0 PID: 2657 at kernel/notifier.c:27
notifier_chain_register+0x5c/0x124 kernel/notifier.c:22
Modules linked in:
CPU: 0 PID: 2657 Comm: syz-executor212 Not tainted
5.10.136-syzkaller-19376-g6f46a5fe0124 #0
8f0771607702f5ef7184d2ee33bd0acd70219fc4
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 07/22/2022
RIP: 0010:notifier_chain_register kernel/notifier.c:27 [inline]
RIP: 0010:notifier_chain_register+0x5c/0x124 kernel/notifier.c:22
Code: 6a 41 00 4c 8b 23 4d 85 e4 0f 84 88 00 00 00 e8 c2 1e 19 00 49
39 ec 75 18 e8 b8 1e 19 00 48 c7 c7 80 6d ca 84 e8 2c 68 48 03 <0f> 0b
e9 af 00 00 00 e8 a0 1e 19 00 48 8d 7d 10 48 89 f8 48 c1 e8
RSP: 0018:ffffc900009d7da8 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8881076fd1d8 RCX: 0000000000000000
RDX: 0000001810895100 RSI: ffff888110895100 RDI: fffff5200013afa7
RBP: ffff88811a4191d0 R08: ffffffff813b8ca1 R09: 0000000080000000
R10: 0000000000000000 R11: 0000000000000005 R12: ffff88811a4191d0
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 00005555571f5300(0000) GS:ffff8881f6c00000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000078e3857f3075 CR3: 000000010d668000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
blocking_notifier_chain_register+0x8c/0xa6 kernel/notifier.c:254
hci_register_suspend_notifier net/bluetooth/hci_core.c:2733
[inline]
hci_register_suspend_notifier+0x6b/0x7c
net/bluetooth/hci_core.c:2727
hci_sock_release+0x270/0x3cf net/bluetooth/hci_sock.c:889
__sock_release+0xcd/0x1de net/socket.c:597
sock_close+0x18/0x1c net/socket.c:1267
__fput+0x418/0x729 fs/file_table.c:281
task_work_run+0x12b/0x15b kernel/task_work.c:151
tracehook_notify_resume include/linux/tracehook.h:188 [inline]
exit_to_user_mode_loop kernel/entry/common.c:165 [inline]
exit_to_user_mode_prepare+0x8f/0x130 kernel/entry/common.c:192
syscall_exit_to_user_mode+0x172/0x1b2 kernel/entry/common.c:268
entry_SYSCALL_64_after_hwframe+0x61/0xc6
RIP: 0033:0x78e38575e1db
Code: 0f 05 48 3d 00 f0 ff ff 77 45 c3 0f 1f 40 00 48 83 ec 18 89
7c 24 0c e8 63 fc ff ff 8b 7c 24 0c 41 89 c0 b8 03 00 00 00 0f 05
<48> 3d 00 f0 ff ff 77 35 44 89 c7 89 44 24 0c e8 a1 fc ff ff 8b 44
RSP: 002b:00007ffffc20a0b0 EFLAGS: 00000293 ORIG_RAX:
0000000000000003
RAX: 0000000000000000 RBX: 0000000000000006 RCX: 000078e38575e1db
RDX: ffffffffffffffb8 RSI: 0000000020000000 RDI: 0000000000000005
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000150
R10: 0000000000000000 R11: 0000000000000293 R12: 000000000000e155
R13: 00007ffffc20a140 R14: 00007ffffc20a130 R15: 00007ffffc20a0e8

Changes in v3:
- No changes.

Changes in v2:
- Removed suspend registration from hci_sock.
- Exit hci_suspend_notifier early if user channel.

net/bluetooth/hci_core.c | 4 ++++
net/bluetooth/hci_sock.c | 3 ---
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 66c7cdba0d32..86ce2dd1c7fb 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2406,6 +2406,10 @@ static int hci_suspend_notifier(struct notifier_block *nb, unsigned long action,
container_of(nb, struct hci_dev, suspend_notifier);
int ret = 0;

+ /* Userspace has full control of this device. Do nothing. */
+ if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL))
+ return NOTIFY_DONE;
+
if (action == PM_SUSPEND_PREPARE)
ret = hci_suspend_dev(hdev);
else if (action == PM_POST_SUSPEND)
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index b2a33a05c93e..06581223238c 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -887,7 +887,6 @@ static int hci_sock_release(struct socket *sock)
*/
hci_dev_do_close(hdev);
hci_dev_clear_flag(hdev, HCI_USER_CHANNEL);
- hci_register_suspend_notifier(hdev);
mgmt_index_added(hdev);
}

@@ -1216,7 +1215,6 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
}

mgmt_index_removed(hdev);
- hci_unregister_suspend_notifier(hdev);

err = hci_dev_open(hdev->id);
if (err) {
@@ -1231,7 +1229,6 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
err = 0;
} else {
hci_dev_clear_flag(hdev, HCI_USER_CHANNEL);
- hci_register_suspend_notifier(hdev);
mgmt_index_added(hdev);
hci_dev_put(hdev);
goto done;
--
2.38.0.rc1.362.ged0d419d3c-goog


2022-09-28 21:40:04

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v3] Bluetooth: Prevent double register of suspend

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

---Test result---

Test Summary:
CheckPatch PASS 2.36 seconds
GitLint PASS 1.10 seconds
SubjectPrefix PASS 1.15 seconds
BuildKernel PASS 33.87 seconds
BuildKernel32 PASS 30.14 seconds
Incremental Build with patchesPASS 44.32 seconds
TestRunner: Setup PASS 505.43 seconds
TestRunner: l2cap-tester PASS 16.76 seconds
TestRunner: iso-tester PASS 15.59 seconds
TestRunner: bnep-tester PASS 6.35 seconds
TestRunner: mgmt-tester PASS 100.48 seconds
TestRunner: rfcomm-tester PASS 9.99 seconds
TestRunner: sco-tester PASS 9.31 seconds
TestRunner: ioctl-tester PASS 10.57 seconds
TestRunner: smp-tester PASS 9.50 seconds
TestRunner: userchan-tester PASS 6.53 seconds



---
Regards,
Linux Bluetooth

2022-09-28 21:55:22

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH v3] Bluetooth: Prevent double register of suspend

Hello:

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

On Wed, 28 Sep 2022 13:00:30 -0700 you wrote:
> From: Abhishek Pandit-Subedi <[email protected]>
>
> Suspend notifier should only be registered and unregistered once per
> hdev. Simplify this by only registering during driver registration and
> simply exiting early when HCI_USER_CHANNEL is set.
>
> Reported-by: syzbot <[email protected]>
> Fixes: 359ee4f834f5 (Bluetooth: Unregister suspend with userchannel)
> Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
>
> [...]

Here is the summary with links:
- [v3] Bluetooth: Prevent double register of suspend
https://git.kernel.org/bluetooth/bluetooth-next/c/4b8af331bb4d

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