* [PATCH] Bluetooth: hci_serdev: call init_rwsem() before p->open()
@ 2022-01-13 13:27 Pavel Skripkin
2022-01-21 22:47 ` Marcel Holtmann
0 siblings, 1 reply; 2+ messages in thread
From: Pavel Skripkin @ 2022-01-13 13:27 UTC (permalink / raw)
To: marcel, johan.hedberg, luiz.dentz, lukas
Cc: linux-bluetooth, linux-kernel, Pavel Skripkin, Yiru Xu
kvartet reported, that hci_uart_tx_wakeup() uses uninitialized rwsem.
The problem was in wrong place for percpu_init_rwsem() call.
hci_uart_proto::open() may register a timer whose callback may call
hci_uart_tx_wakeup(). There is a chance, that hci_uart_register_device()
thread won't be fast enough to call percpu_init_rwsem().
Fix it my moving percpu_init_rwsem() call before p->open().
INFO: trying to register non-static key.
The code is fine but needs lockdep annotation, or maybe
you didn't initialize this object before use?
turning off the locking correctness validator.
CPU: 2 PID: 18524 Comm: syz-executor.5 Not tainted 5.16.0-rc6 #9
...
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
assign_lock_key kernel/locking/lockdep.c:951 [inline]
register_lock_class+0x148d/0x1950 kernel/locking/lockdep.c:1263
__lock_acquire+0x106/0x57e0 kernel/locking/lockdep.c:4906
lock_acquire kernel/locking/lockdep.c:5637 [inline]
lock_acquire+0x1ab/0x520 kernel/locking/lockdep.c:5602
percpu_down_read_trylock include/linux/percpu-rwsem.h:92 [inline]
hci_uart_tx_wakeup+0x12e/0x490 drivers/bluetooth/hci_ldisc.c:124
h5_timed_event+0x32f/0x6a0 drivers/bluetooth/hci_h5.c:188
call_timer_fn+0x1a5/0x6b0 kernel/time/timer.c:1421
Fixes: d73e17281665 ("Bluetooth: hci_serdev: Init hci_uart proto_lock to avoid oops")
Reported-by: Yiru Xu <xyru1999@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/bluetooth/hci_serdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 9e03402ef1b3..e9a44ab3812d 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -305,6 +305,8 @@ int hci_uart_register_device(struct hci_uart *hu,
if (err)
return err;
+ percpu_init_rwsem(&hu->proto_lock);
+
err = p->open(hu);
if (err)
goto err_open;
@@ -327,7 +329,6 @@ int hci_uart_register_device(struct hci_uart *hu,
INIT_WORK(&hu->init_ready, hci_uart_init_work);
INIT_WORK(&hu->write_work, hci_uart_write_work);
- percpu_init_rwsem(&hu->proto_lock);
/* Only when vendor specific setup callback is provided, consider
* the manufacturer information valid. This avoids filling in the
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] Bluetooth: hci_serdev: call init_rwsem() before p->open()
2022-01-13 13:27 [PATCH] Bluetooth: hci_serdev: call init_rwsem() before p->open() Pavel Skripkin
@ 2022-01-21 22:47 ` Marcel Holtmann
0 siblings, 0 replies; 2+ messages in thread
From: Marcel Holtmann @ 2022-01-21 22:47 UTC (permalink / raw)
To: Pavel Skripkin
Cc: Johan Hedberg, Luiz Augusto von Dentz, lukas, BlueZ,
linux-kernel, Yiru Xu
Hi Pavel,
> kvartet reported, that hci_uart_tx_wakeup() uses uninitialized rwsem.
> The problem was in wrong place for percpu_init_rwsem() call.
>
> hci_uart_proto::open() may register a timer whose callback may call
> hci_uart_tx_wakeup(). There is a chance, that hci_uart_register_device()
> thread won't be fast enough to call percpu_init_rwsem().
>
> Fix it my moving percpu_init_rwsem() call before p->open().
>
> INFO: trying to register non-static key.
> The code is fine but needs lockdep annotation, or maybe
> you didn't initialize this object before use?
> turning off the locking correctness validator.
> CPU: 2 PID: 18524 Comm: syz-executor.5 Not tainted 5.16.0-rc6 #9
> ...
> Call Trace:
> <IRQ>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> assign_lock_key kernel/locking/lockdep.c:951 [inline]
> register_lock_class+0x148d/0x1950 kernel/locking/lockdep.c:1263
> __lock_acquire+0x106/0x57e0 kernel/locking/lockdep.c:4906
> lock_acquire kernel/locking/lockdep.c:5637 [inline]
> lock_acquire+0x1ab/0x520 kernel/locking/lockdep.c:5602
> percpu_down_read_trylock include/linux/percpu-rwsem.h:92 [inline]
> hci_uart_tx_wakeup+0x12e/0x490 drivers/bluetooth/hci_ldisc.c:124
> h5_timed_event+0x32f/0x6a0 drivers/bluetooth/hci_h5.c:188
> call_timer_fn+0x1a5/0x6b0 kernel/time/timer.c:1421
>
> Fixes: d73e17281665 ("Bluetooth: hci_serdev: Init hci_uart proto_lock to avoid oops")
> Reported-by: Yiru Xu <xyru1999@gmail.com>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> drivers/bluetooth/hci_serdev.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-21 22:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 13:27 [PATCH] Bluetooth: hci_serdev: call init_rwsem() before p->open() Pavel Skripkin
2022-01-21 22:47 ` Marcel Holtmann
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).