Message ID | 20220113132704.4354-1-paskripkin@gmail.com |
---|---|
State | Accepted |
Commit | 9d7cbe2b9cf5f650067df4f402fdd799d4bbb4e1 |
Headers | show |
Series | Bluetooth: hci_serdev: call init_rwsem() before p->open() | expand |
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
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
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(-)