lkml.org 
[lkml]   [2021]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Two issues caused by commit e305509e678b3 ("Bluetooth: use correct lock to prevent UAF of hdev object")
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
\
 
 \ /
  Last update: 2021-07-22 05:00    [W:0.056 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site