Re: INFO: trying to register non-static key in hci_uart_tx_wakeup

[Pavel Skripkin]

On 1/4/22 11:49, kvartet wrote:
> Hello,
>
> When using Syzkaller to fuzz the latest Linux kernel, the following
> crash was triggered.
>
> HEAD commit: a7904a538933 Linux 5.16-rc6
> git tree: upstream
> console output: https://paste.ubuntu.com/p/Bfpr8Gxtd4/plain/
> kernel config: https://paste.ubuntu.com/p/FDDNHDxtwz/plain/
>
> Sorry, I don't have a reproducer for this crash, hope the symbolized
> report can help.
> If you fix this issue, please add the following tag to the commit:
> Reported-by: Yiru Xu <xyru1999@xxxxxxxxx>
>
>
> 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
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.13.0-1ubuntu1.1 04/01/2014
> 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

hci_uart_proto.open() functions register timer, which may call 
hci_uart_tx_wakeup(), so we need to initalize rwsem before calling
hci_uart_proto.open()

Just for thoughts


With regards,
Pavel Skripkin
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 3b00d82d36cf..4cda890ce647 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