* Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object")
@ 2021-07-13 17:36 Jiri Kosina
2021-07-21 18:26 ` Jiri Kosina
0 siblings, 1 reply; 4+ messages in thread
From: Jiri Kosina @ 2021-07-13 17:36 UTC (permalink / raw)
To: Lin Ma, Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz
Cc: linux-bluetooth, linux-kernel
Hi,
commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev
object") has two issues, and I believe it should be reverted for 5.14
final. Could you please elaborate what is the exact UAF scenario the patch
is fixing? It's rather hard to deduce from the very short changelog, but
it's pretty obvious that it creates at least two issues.
(1) it introduces a possibility of deadlock between hci_sock_dev_event()
and hci_sock_sendmsg().
Namely:
- hci_sock_sendmsg(HCI_CHANNEL_LOGGING) acquires lock_sock(sk) and then
calls into hci_logging_frame() -> hci_send_to_channel() which in turn
acquires hci_sk_list.lock
- after the mentioned commit, hci_sock_dev_event() first acquires
hci_sk_list.lock before doing lock_sock(sk) on each of the sockets it
iterates through, creating the reverse dependency
Please find the full lockdep report below for reference.
(2) it causes sleeping function to be called from atomic context, because
it's not allowed to sleep after acquiring read_lock(), which is exactly
what this patch does (lock_sock() is sleepable). Report below as well.
BUG: sleeping function called from invalid context at net/core/sock.c:3100
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 448, name: kworker/0:5
6 locks held by kworker/0:5/448:
#0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
#1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
#2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
#3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
#4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
#5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
CPU: 0 PID: 448 Comm: kworker/0:5 Not tainted 5.14.0-rc1-00003-g7fef2edf7cc7 #15
Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
Workqueue: usb_hub_wq hub_event [usbcore]
Call Trace:
dump_stack_lvl+0x56/0x6c
___might_sleep+0x1b6/0x210
lock_sock_nested+0x29/0xa0
hci_sock_dev_event+0x156/0x1e0 [bluetooth]
hci_unregister_dev+0xd2/0x350 [bluetooth]
btusb_disconnect+0x63/0x150 [btusb]
usb_unbind_interface+0x79/0x260 [usbcore]
device_release_driver_internal+0xf7/0x1d0
bus_remove_device+0xef/0x160
device_del+0x1ac/0x430
? usb_remove_ep_devs+0x1b/0x30 [usbcore]
usb_disable_device+0x8d/0x1a0 [usbcore]
usb_disconnect+0xc1/0x270 [usbcore]
? hub_event+0x4b0/0xd50 [usbcore]
hub_port_connect+0x82/0xa20 [usbcore]
? __mutex_unlock_slowpath+0x43/0x2b0
hub_event+0x4c4/0xd50 [usbcore]
? lock_is_held_type+0xb4/0x120
process_one_work+0x244/0x5e0
worker_thread+0x3c/0x390
? process_one_work+0x5e0/0x5e0
kthread+0x133/0x160
? set_kthread_struct+0x40/0x40
ret_from_fork+0x22/0x30
======================================================
WARNING: possible circular locking dependency detected
5.14.0-rc1-00003-g7fef2edf7cc7 #15 Tainted: G W
------------------------------------------------------
kworker/0:5/448 is trying to acquire lock:
ffff89e950647920 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}, at: hci_sock_dev_event+0x156/0x1e0 [bluetooth]
but task is already holding lock:
ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (hci_sk_list.lock){++++}-{2:2}:
_raw_read_lock+0x38/0x70
systemd-sysv-generator[1254]: stat() failed on /etc/init.d/jexec, ignoring: No such file or directory
hci_send_to_channel+0x22/0x50 [bluetooth]
hci_sock_sendmsg+0xa2b/0xa40 [bluetooth]
sock_sendmsg+0x5b/0x60
____sys_sendmsg+0x1ed/0x250
systemd-sysv-generator[1254]: SysV service '/etc/init.d/tpfand' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
___sys_sendmsg+0x88/0xd0
__sys_sendmsg+0x5e/0xa0
do_syscall_64+0x3a/0xb0
systemd-sysv-generator[1254]: SysV service '/etc/init.d/boot.local' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}:
__lock_acquire+0x124a/0x1680
lock_acquire+0x278/0x300
lock_sock_nested+0x72/0xa0
hci_sock_dev_event+0x156/0x1e0 [bluetooth]
hci_unregister_dev+0xd2/0x350 [bluetooth]
btusb_disconnect+0x63/0x150 [btusb]
usb_unbind_interface+0x79/0x260 [usbcore]
device_release_driver_internal+0xf7/0x1d0
bus_remove_device+0xef/0x160
device_del+0x1ac/0x430
usb_disable_device+0x8d/0x1a0 [usbcore]
usb_disconnect+0xc1/0x270 [usbcore]
hub_port_connect+0x82/0xa20 [usbcore]
hub_event+0x4c4/0xd50 [usbcore]
process_one_work+0x244/0x5e0
worker_thread+0x3c/0x390
kthread+0x133/0x160
ret_from_fork+0x22/0x30
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(hci_sk_list.lock);
lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
lock(hci_sk_list.lock);
lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
*** DEADLOCK ***
6 locks held by kworker/0:5/448:
#0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
#1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
#2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
#3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
#4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
#5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
stack backtrace:
CPU: 0 PID: 448 Comm: kworker/0:5 Tainted: G W 5.14.0-rc1-00003-g7fef2edf7cc7 #15
Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
Workqueue: usb_hub_wq hub_event [usbcore]
Call Trace:
dump_stack_lvl+0x56/0x6c
check_noncircular+0x105/0x120
? stack_trace_save+0x4b/0x70
? save_trace+0x3d/0x340
? __lock_acquire+0x124a/0x1680
__lock_acquire+0x124a/0x1680
lock_acquire+0x278/0x300
? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
? lock_release+0x15a/0x2a0
lock_sock_nested+0x72/0xa0
? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
hci_sock_dev_event+0x156/0x1e0 [bluetooth]
hci_unregister_dev+0xd2/0x350 [bluetooth]
btusb_disconnect+0x63/0x150 [btusb]
usb_unbind_interface+0x79/0x260 [usbcore]
device_release_driver_internal+0xf7/0x1d0
bus_remove_device+0xef/0x160
device_del+0x1ac/0x430
? usb_remove_ep_devs+0x1b/0x30 [usbcore]
usb_disable_device+0x8d/0x1a0 [usbcore]
usb_disconnect+0xc1/0x270 [usbcore]
? hub_event+0x4b0/0xd50 [usbcore]
hub_port_connect+0x82/0xa20 [usbcore]
? __mutex_unlock_slowpath+0x43/0x2b0
hub_event+0x4c4/0xd50 [usbcore]
? lock_is_held_type+0xb4/0x120
process_one_work+0x244/0x5e0
worker_thread+0x3c/0x390
? process_one_work+0x5e0/0x5e0
kthread+0x133/0x160
? set_kthread_struct+0x40/0x40
ret_from_fork+0x22/0x30
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object")
2021-07-13 17:36 Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object") Jiri Kosina
@ 2021-07-21 18:26 ` Jiri Kosina
2021-07-21 18:31 ` Luiz Augusto von Dentz
2021-07-22 3:00 ` LinMa
0 siblings, 2 replies; 4+ messages in thread
From: Jiri Kosina @ 2021-07-21 18:26 UTC (permalink / raw)
To: Lin Ma, Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz
Cc: linux-bluetooth, linux-kernel
On Tue, 13 Jul 2021, Jiri Kosina wrote:
> Hi,
>
> commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev
> object") has two issues, and I believe it should be reverted for 5.14
> final. Could you please elaborate what is the exact UAF scenario the patch
> is fixing? It's rather hard to deduce from the very short changelog, but
> it's pretty obvious that it creates at least two issues.
Marcel, Johan, Luiz, any thoughts on this please? Thanks.
>
>
>
> (1) it introduces a possibility of deadlock between hci_sock_dev_event()
> and hci_sock_sendmsg().
>
> Namely:
>
> - hci_sock_sendmsg(HCI_CHANNEL_LOGGING) acquires lock_sock(sk) and then
> calls into hci_logging_frame() -> hci_send_to_channel() which in turn
> acquires hci_sk_list.lock
>
> - after the mentioned commit, hci_sock_dev_event() first acquires
> hci_sk_list.lock before doing lock_sock(sk) on each of the sockets it
> iterates through, creating the reverse dependency
>
>
> Please find the full lockdep report below for reference.
>
> (2) it causes sleeping function to be called from atomic context, because
> it's not allowed to sleep after acquiring read_lock(), which is exactly
> what this patch does (lock_sock() is sleepable). Report below as well.
>
>
>
>
>
>
> BUG: sleeping function called from invalid context at net/core/sock.c:3100
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 448, name: kworker/0:5
> 6 locks held by kworker/0:5/448:
> #0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> #1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> #2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
> #3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
> #4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
> #5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> CPU: 0 PID: 448 Comm: kworker/0:5 Not tainted 5.14.0-rc1-00003-g7fef2edf7cc7 #15
> Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> Workqueue: usb_hub_wq hub_event [usbcore]
> Call Trace:
> dump_stack_lvl+0x56/0x6c
> ___might_sleep+0x1b6/0x210
> lock_sock_nested+0x29/0xa0
> hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> hci_unregister_dev+0xd2/0x350 [bluetooth]
> btusb_disconnect+0x63/0x150 [btusb]
> usb_unbind_interface+0x79/0x260 [usbcore]
> device_release_driver_internal+0xf7/0x1d0
> bus_remove_device+0xef/0x160
> device_del+0x1ac/0x430
> ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> usb_disable_device+0x8d/0x1a0 [usbcore]
> usb_disconnect+0xc1/0x270 [usbcore]
> ? hub_event+0x4b0/0xd50 [usbcore]
> hub_port_connect+0x82/0xa20 [usbcore]
> ? __mutex_unlock_slowpath+0x43/0x2b0
> hub_event+0x4c4/0xd50 [usbcore]
> ? lock_is_held_type+0xb4/0x120
> process_one_work+0x244/0x5e0
> worker_thread+0x3c/0x390
> ? process_one_work+0x5e0/0x5e0
> kthread+0x133/0x160
> ? set_kthread_struct+0x40/0x40
> ret_from_fork+0x22/0x30
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 5.14.0-rc1-00003-g7fef2edf7cc7 #15 Tainted: G W
> ------------------------------------------------------
> kworker/0:5/448 is trying to acquire lock:
> ffff89e950647920 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}, at: hci_sock_dev_event+0x156/0x1e0 [bluetooth]
>
> but task is already holding lock:
> ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (hci_sk_list.lock){++++}-{2:2}:
> _raw_read_lock+0x38/0x70
> systemd-sysv-generator[1254]: stat() failed on /etc/init.d/jexec, ignoring: No such file or directory
> hci_send_to_channel+0x22/0x50 [bluetooth]
> hci_sock_sendmsg+0xa2b/0xa40 [bluetooth]
> sock_sendmsg+0x5b/0x60
> ____sys_sendmsg+0x1ed/0x250
> systemd-sysv-generator[1254]: SysV service '/etc/init.d/tpfand' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
> ___sys_sendmsg+0x88/0xd0
> __sys_sendmsg+0x5e/0xa0
> do_syscall_64+0x3a/0xb0
> systemd-sysv-generator[1254]: SysV service '/etc/init.d/boot.local' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> -> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}:
> __lock_acquire+0x124a/0x1680
> lock_acquire+0x278/0x300
> lock_sock_nested+0x72/0xa0
> hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> hci_unregister_dev+0xd2/0x350 [bluetooth]
> btusb_disconnect+0x63/0x150 [btusb]
> usb_unbind_interface+0x79/0x260 [usbcore]
> device_release_driver_internal+0xf7/0x1d0
> bus_remove_device+0xef/0x160
> device_del+0x1ac/0x430
> usb_disable_device+0x8d/0x1a0 [usbcore]
> usb_disconnect+0xc1/0x270 [usbcore]
> hub_port_connect+0x82/0xa20 [usbcore]
> hub_event+0x4c4/0xd50 [usbcore]
> process_one_work+0x244/0x5e0
> worker_thread+0x3c/0x390
> kthread+0x133/0x160
> ret_from_fork+0x22/0x30
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(hci_sk_list.lock);
> lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
> lock(hci_sk_list.lock);
> lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
>
> *** DEADLOCK ***
>
> 6 locks held by kworker/0:5/448:
> #0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> #1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> #2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
> #3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
> #4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
> #5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
>
> stack backtrace:
> CPU: 0 PID: 448 Comm: kworker/0:5 Tainted: G W 5.14.0-rc1-00003-g7fef2edf7cc7 #15
> Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> Workqueue: usb_hub_wq hub_event [usbcore]
> Call Trace:
> dump_stack_lvl+0x56/0x6c
> check_noncircular+0x105/0x120
> ? stack_trace_save+0x4b/0x70
> ? save_trace+0x3d/0x340
> ? __lock_acquire+0x124a/0x1680
> __lock_acquire+0x124a/0x1680
> lock_acquire+0x278/0x300
> ? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> ? lock_release+0x15a/0x2a0
> lock_sock_nested+0x72/0xa0
> ? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> hci_unregister_dev+0xd2/0x350 [bluetooth]
> btusb_disconnect+0x63/0x150 [btusb]
> usb_unbind_interface+0x79/0x260 [usbcore]
> device_release_driver_internal+0xf7/0x1d0
> bus_remove_device+0xef/0x160
> device_del+0x1ac/0x430
> ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> usb_disable_device+0x8d/0x1a0 [usbcore]
> usb_disconnect+0xc1/0x270 [usbcore]
> ? hub_event+0x4b0/0xd50 [usbcore]
> hub_port_connect+0x82/0xa20 [usbcore]
> ? __mutex_unlock_slowpath+0x43/0x2b0
> hub_event+0x4c4/0xd50 [usbcore]
> ? lock_is_held_type+0xb4/0x120
> process_one_work+0x244/0x5e0
> worker_thread+0x3c/0x390
> ? process_one_work+0x5e0/0x5e0
> kthread+0x133/0x160
> ? set_kthread_struct+0x40/0x40
> ret_from_fork+0x22/0x30
>
>
>
> --
> Jiri Kosina
> SUSE Labs
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object")
2021-07-21 18:26 ` Jiri Kosina
@ 2021-07-21 18:31 ` Luiz Augusto von Dentz
2021-07-22 3:00 ` LinMa
1 sibling, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2021-07-21 18:31 UTC (permalink / raw)
To: Jiri Kosina
Cc: Lin Ma, Marcel Holtmann, Johan Hedberg, linux-bluetooth,
Linux Kernel Mailing List
Hi Jiri,
On Wed, Jul 21, 2021 at 11:26 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Tue, 13 Jul 2021, Jiri Kosina wrote:
>
> > Hi,
> >
> > commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev
> > object") has two issues, and I believe it should be reverted for 5.14
> > final. Could you please elaborate what is the exact UAF scenario the patch
> > is fixing? It's rather hard to deduce from the very short changelog, but
> > it's pretty obvious that it creates at least two issues.
>
> Marcel, Johan, Luiz, any thoughts on this please? Thanks.
We are looking into it.
> >
> >
> >
> > (1) it introduces a possibility of deadlock between hci_sock_dev_event()
> > and hci_sock_sendmsg().
> >
> > Namely:
> >
> > - hci_sock_sendmsg(HCI_CHANNEL_LOGGING) acquires lock_sock(sk) and then
> > calls into hci_logging_frame() -> hci_send_to_channel() which in turn
> > acquires hci_sk_list.lock
> >
> > - after the mentioned commit, hci_sock_dev_event() first acquires
> > hci_sk_list.lock before doing lock_sock(sk) on each of the sockets it
> > iterates through, creating the reverse dependency
> >
> >
> > Please find the full lockdep report below for reference.
> >
> > (2) it causes sleeping function to be called from atomic context, because
> > it's not allowed to sleep after acquiring read_lock(), which is exactly
> > what this patch does (lock_sock() is sleepable). Report below as well.
> >
> >
> >
> >
> >
> >
> > BUG: sleeping function called from invalid context at net/core/sock.c:3100
> > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 448, name: kworker/0:5
> > 6 locks held by kworker/0:5/448:
> > #0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
> > #3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
> > #4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
> > #5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> > CPU: 0 PID: 448 Comm: kworker/0:5 Not tainted 5.14.0-rc1-00003-g7fef2edf7cc7 #15
> > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> > Workqueue: usb_hub_wq hub_event [usbcore]
> > Call Trace:
> > dump_stack_lvl+0x56/0x6c
> > ___might_sleep+0x1b6/0x210
> > lock_sock_nested+0x29/0xa0
> > hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_unregister_dev+0xd2/0x350 [bluetooth]
> > btusb_disconnect+0x63/0x150 [btusb]
> > usb_unbind_interface+0x79/0x260 [usbcore]
> > device_release_driver_internal+0xf7/0x1d0
> > bus_remove_device+0xef/0x160
> > device_del+0x1ac/0x430
> > ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> > usb_disable_device+0x8d/0x1a0 [usbcore]
> > usb_disconnect+0xc1/0x270 [usbcore]
> > ? hub_event+0x4b0/0xd50 [usbcore]
> > hub_port_connect+0x82/0xa20 [usbcore]
> > ? __mutex_unlock_slowpath+0x43/0x2b0
> > hub_event+0x4c4/0xd50 [usbcore]
> > ? lock_is_held_type+0xb4/0x120
> > process_one_work+0x244/0x5e0
> > worker_thread+0x3c/0x390
> > ? process_one_work+0x5e0/0x5e0
> > kthread+0x133/0x160
> > ? set_kthread_struct+0x40/0x40
> > ret_from_fork+0x22/0x30
> >
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 5.14.0-rc1-00003-g7fef2edf7cc7 #15 Tainted: G W
> > ------------------------------------------------------
> > kworker/0:5/448 is trying to acquire lock:
> > ffff89e950647920 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}, at: hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> >
> > but task is already holding lock:
> > ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> >
> > which lock already depends on the new lock.
> >
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #1 (hci_sk_list.lock){++++}-{2:2}:
> > _raw_read_lock+0x38/0x70
> > systemd-sysv-generator[1254]: stat() failed on /etc/init.d/jexec, ignoring: No such file or directory
> > hci_send_to_channel+0x22/0x50 [bluetooth]
> > hci_sock_sendmsg+0xa2b/0xa40 [bluetooth]
> > sock_sendmsg+0x5b/0x60
> > ____sys_sendmsg+0x1ed/0x250
> > systemd-sysv-generator[1254]: SysV service '/etc/init.d/tpfand' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
> > ___sys_sendmsg+0x88/0xd0
> > __sys_sendmsg+0x5e/0xa0
> > do_syscall_64+0x3a/0xb0
> > systemd-sysv-generator[1254]: SysV service '/etc/init.d/boot.local' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
> > entry_SYSCALL_64_after_hwframe+0x44/0xae
> >
> > -> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}:
> > __lock_acquire+0x124a/0x1680
> > lock_acquire+0x278/0x300
> > lock_sock_nested+0x72/0xa0
> > hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_unregister_dev+0xd2/0x350 [bluetooth]
> > btusb_disconnect+0x63/0x150 [btusb]
> > usb_unbind_interface+0x79/0x260 [usbcore]
> > device_release_driver_internal+0xf7/0x1d0
> > bus_remove_device+0xef/0x160
> > device_del+0x1ac/0x430
> > usb_disable_device+0x8d/0x1a0 [usbcore]
> > usb_disconnect+0xc1/0x270 [usbcore]
> > hub_port_connect+0x82/0xa20 [usbcore]
> > hub_event+0x4c4/0xd50 [usbcore]
> > process_one_work+0x244/0x5e0
> > worker_thread+0x3c/0x390
> > kthread+0x133/0x160
> > ret_from_fork+0x22/0x30
> >
> > other info that might help us debug this:
> >
> > Possible unsafe locking scenario:
> >
> > CPU0 CPU1
> > ---- ----
> > lock(hci_sk_list.lock);
> > lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
> > lock(hci_sk_list.lock);
> > lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
> >
> > *** DEADLOCK ***
> >
> > 6 locks held by kworker/0:5/448:
> > #0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
> > #3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
> > #4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
> > #5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> >
> > stack backtrace:
> > CPU: 0 PID: 448 Comm: kworker/0:5 Tainted: G W 5.14.0-rc1-00003-g7fef2edf7cc7 #15
> > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> > Workqueue: usb_hub_wq hub_event [usbcore]
> > Call Trace:
> > dump_stack_lvl+0x56/0x6c
> > check_noncircular+0x105/0x120
> > ? stack_trace_save+0x4b/0x70
> > ? save_trace+0x3d/0x340
> > ? __lock_acquire+0x124a/0x1680
> > __lock_acquire+0x124a/0x1680
> > lock_acquire+0x278/0x300
> > ? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > ? lock_release+0x15a/0x2a0
> > lock_sock_nested+0x72/0xa0
> > ? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_unregister_dev+0xd2/0x350 [bluetooth]
> > btusb_disconnect+0x63/0x150 [btusb]
> > usb_unbind_interface+0x79/0x260 [usbcore]
> > device_release_driver_internal+0xf7/0x1d0
> > bus_remove_device+0xef/0x160
> > device_del+0x1ac/0x430
> > ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> > usb_disable_device+0x8d/0x1a0 [usbcore]
> > usb_disconnect+0xc1/0x270 [usbcore]
> > ? hub_event+0x4b0/0xd50 [usbcore]
> > hub_port_connect+0x82/0xa20 [usbcore]
> > ? __mutex_unlock_slowpath+0x43/0x2b0
> > hub_event+0x4c4/0xd50 [usbcore]
> > ? lock_is_held_type+0xb4/0x120
> > process_one_work+0x244/0x5e0
> > worker_thread+0x3c/0x390
> > ? process_one_work+0x5e0/0x5e0
> > kthread+0x133/0x160
> > ? set_kthread_struct+0x40/0x40
> > ret_from_fork+0x22/0x30
> >
> >
> >
> > --
> > Jiri Kosina
> > SUSE Labs
> >
>
> --
> Jiri Kosina
> SUSE Labs
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object")
2021-07-21 18:26 ` Jiri Kosina
2021-07-21 18:31 ` Luiz Augusto von Dentz
@ 2021-07-22 3:00 ` LinMa
1 sibling, 0 replies; 4+ messages in thread
From: LinMa @ 2021-07-22 3:00 UTC (permalink / raw)
To: Jiri Kosina
Cc: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
linux-bluetooth, linux-kernel
Hi Jiri
Thanks for the concern. In fact, the discussion over this issue has been several weeks, and we welcome new thoughts.
> From: "Jiri Kosina" <jikos@kernel.org>
> Sent Time: 2021-07-22 02:26:20 (Thursday)
> To: "Lin Ma" <linma@zju.edu.cn>, "Marcel Holtmann" <marcel@holtmann.org>, "Johan Hedberg" <johan.hedberg@gmail.com>, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object")
>
> On Tue, 13 Jul 2021, Jiri Kosina wrote:
>
> > Hi,
> >
> > commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev
> > object") has two issues, and I believe it should be reverted for 5.14
> > final. Could you please elaborate what is the exact UAF scenario the patch
> > is fixing? It's rather hard to deduce from the very short changelog, but
> > it's pretty obvious that it creates at least two issues.
>
> Marcel, Johan, Luiz, any thoughts on this please? Thanks.
>
The details of this UAF can be found at oss: https://www.openwall.com/lists/oss-security/2021/06/08/2
And you can follow some existing discussions at:
https://www.spinics.net/lists/linux-bluetooth/msg92649.html
https://marc.info/?l=linux-bluetooth&m=162441276805113&w=2
https://marc.info/?l=linux-bluetooth&m=162653675414330&w=2
Hope this can help you to understand the problem we are confronting.
Thanks
Lin Ma
> >
> >
> >
> > (1) it introduces a possibility of deadlock between hci_sock_dev_event()
> > and hci_sock_sendmsg().
> >
> > Namely:
> >
> > - hci_sock_sendmsg(HCI_CHANNEL_LOGGING) acquires lock_sock(sk) and then
> > calls into hci_logging_frame() -> hci_send_to_channel() which in turn
> > acquires hci_sk_list.lock
> >
> > - after the mentioned commit, hci_sock_dev_event() first acquires
> > hci_sk_list.lock before doing lock_sock(sk) on each of the sockets it
> > iterates through, creating the reverse dependency
> >
> >
> > Please find the full lockdep report below for reference.
> >
> > (2) it causes sleeping function to be called from atomic context, because
> > it's not allowed to sleep after acquiring read_lock(), which is exactly
> > what this patch does (lock_sock() is sleepable). Report below as well.
> >
> >
> >
> >
> >
> >
> > BUG: sleeping function called from invalid context at net/core/sock.c:3100
> > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 448, name: kworker/0:5
> > 6 locks held by kworker/0:5/448:
> > #0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
> > #3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
> > #4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
> > #5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> > CPU: 0 PID: 448 Comm: kworker/0:5 Not tainted 5.14.0-rc1-00003-g7fef2edf7cc7 #15
> > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> > Workqueue: usb_hub_wq hub_event [usbcore]
> > Call Trace:
> > dump_stack_lvl+0x56/0x6c
> > ___might_sleep+0x1b6/0x210
> > lock_sock_nested+0x29/0xa0
> > hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_unregister_dev+0xd2/0x350 [bluetooth]
> > btusb_disconnect+0x63/0x150 [btusb]
> > usb_unbind_interface+0x79/0x260 [usbcore]
> > device_release_driver_internal+0xf7/0x1d0
> > bus_remove_device+0xef/0x160
> > device_del+0x1ac/0x430
> > ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> > usb_disable_device+0x8d/0x1a0 [usbcore]
> > usb_disconnect+0xc1/0x270 [usbcore]
> > ? hub_event+0x4b0/0xd50 [usbcore]
> > hub_port_connect+0x82/0xa20 [usbcore]
> > ? __mutex_unlock_slowpath+0x43/0x2b0
> > hub_event+0x4c4/0xd50 [usbcore]
> > ? lock_is_held_type+0xb4/0x120
> > process_one_work+0x244/0x5e0
> > worker_thread+0x3c/0x390
> > ? process_one_work+0x5e0/0x5e0
> > kthread+0x133/0x160
> > ? set_kthread_struct+0x40/0x40
> > ret_from_fork+0x22/0x30
> >
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 5.14.0-rc1-00003-g7fef2edf7cc7 #15 Tainted: G W
> > ------------------------------------------------------
> > kworker/0:5/448 is trying to acquire lock:
> > ffff89e950647920 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}, at: hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> >
> > but task is already holding lock:
> > ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> >
> > which lock already depends on the new lock.
> >
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #1 (hci_sk_list.lock){++++}-{2:2}:
> > _raw_read_lock+0x38/0x70
> > systemd-sysv-generator[1254]: stat() failed on /etc/init.d/jexec, ignoring: No such file or directory
> > hci_send_to_channel+0x22/0x50 [bluetooth]
> > hci_sock_sendmsg+0xa2b/0xa40 [bluetooth]
> > sock_sendmsg+0x5b/0x60
> > ____sys_sendmsg+0x1ed/0x250
> > systemd-sysv-generator[1254]: SysV service '/etc/init.d/tpfand' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
> > ___sys_sendmsg+0x88/0xd0
> > __sys_sendmsg+0x5e/0xa0
> > do_syscall_64+0x3a/0xb0
> > systemd-sysv-generator[1254]: SysV service '/etc/init.d/boot.local' lacks a native systemd unit file. Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it more safe and robust.
> > entry_SYSCALL_64_after_hwframe+0x44/0xae
> >
> > -> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_HCI){+.+.}-{0:0}:
> > __lock_acquire+0x124a/0x1680
> > lock_acquire+0x278/0x300
> > lock_sock_nested+0x72/0xa0
> > hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_unregister_dev+0xd2/0x350 [bluetooth]
> > btusb_disconnect+0x63/0x150 [btusb]
> > usb_unbind_interface+0x79/0x260 [usbcore]
> > device_release_driver_internal+0xf7/0x1d0
> > bus_remove_device+0xef/0x160
> > device_del+0x1ac/0x430
> > usb_disable_device+0x8d/0x1a0 [usbcore]
> > usb_disconnect+0xc1/0x270 [usbcore]
> > hub_port_connect+0x82/0xa20 [usbcore]
> > hub_event+0x4c4/0xd50 [usbcore]
> > process_one_work+0x244/0x5e0
> > worker_thread+0x3c/0x390
> > kthread+0x133/0x160
> > ret_from_fork+0x22/0x30
> >
> > other info that might help us debug this:
> >
> > Possible unsafe locking scenario:
> >
> > CPU0 CPU1
> > ---- ----
> > lock(hci_sk_list.lock);
> > lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
> > lock(hci_sk_list.lock);
> > lock(sk_lock-AF_BLUETOOTH-BTPROTO_HCI);
> >
> > *** DEADLOCK ***
> >
> > 6 locks held by kworker/0:5/448:
> > #0: ffff89e947fb4338 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #1: ffffadef40973e78 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x1c2/0x5e0
> > #2: ffff89e946998a20 (&dev->mutex){....}-{3:3}, at: hub_event+0x6a/0xd50 [usbcore]
> > #3: ffff89e950828a20 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x54/0x270 [usbcore]
> > #4: ffff89e9504e71a8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x1a/0x1d0
> > #5: ffffffffc117a3c0 (hci_sk_list.lock){++++}-{2:2}, at: hci_sock_dev_event+0x12b/0x1e0 [bluetooth]
> >
> > stack backtrace:
> > CPU: 0 PID: 448 Comm: kworker/0:5 Tainted: G W 5.14.0-rc1-00003-g7fef2edf7cc7 #15
> > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> > Workqueue: usb_hub_wq hub_event [usbcore]
> > Call Trace:
> > dump_stack_lvl+0x56/0x6c
> > check_noncircular+0x105/0x120
> > ? stack_trace_save+0x4b/0x70
> > ? save_trace+0x3d/0x340
> > ? __lock_acquire+0x124a/0x1680
> > __lock_acquire+0x124a/0x1680
> > lock_acquire+0x278/0x300
> > ? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > ? lock_release+0x15a/0x2a0
> > lock_sock_nested+0x72/0xa0
> > ? hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_sock_dev_event+0x156/0x1e0 [bluetooth]
> > hci_unregister_dev+0xd2/0x350 [bluetooth]
> > btusb_disconnect+0x63/0x150 [btusb]
> > usb_unbind_interface+0x79/0x260 [usbcore]
> > device_release_driver_internal+0xf7/0x1d0
> > bus_remove_device+0xef/0x160
> > device_del+0x1ac/0x430
> > ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> > usb_disable_device+0x8d/0x1a0 [usbcore]
> > usb_disconnect+0xc1/0x270 [usbcore]
> > ? hub_event+0x4b0/0xd50 [usbcore]
> > hub_port_connect+0x82/0xa20 [usbcore]
> > ? __mutex_unlock_slowpath+0x43/0x2b0
> > hub_event+0x4c4/0xd50 [usbcore]
> > ? lock_is_held_type+0xb4/0x120
> > process_one_work+0x244/0x5e0
> > worker_thread+0x3c/0x390
> > ? process_one_work+0x5e0/0x5e0
> > kthread+0x133/0x160
> > ? set_kthread_struct+0x40/0x40
> > ret_from_fork+0x22/0x30
> >
> >
> >
> > --
> > Jiri Kosina
> > SUSE Labs
> >
>
> --
> Jiri Kosina
> SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-22 3:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 17:36 Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object") Jiri Kosina
2021-07-21 18:26 ` Jiri Kosina
2021-07-21 18:31 ` Luiz Augusto von Dentz
2021-07-22 3:00 ` LinMa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).