[PATCH] [RFC] Bluetooth: hci_ldisc: Possible race condition

[Marek Vasut <marex@denx.de>]

I suspect the following race condition happens on system reboot,
when using the TI UART bluetooth adapter, attached using:

$ hciattach -n /dev/ttymxc0 texas 115200 flow

In drivers/bluetooth/hci_ldisc.c:
1) write_work is canceled in hci_uart_tty_close() using cancel_work_sync()
2) transmission is scheduled right after the work is canceled using
   hci_uart_send_frame()
3) protocol is closed in hci_uart_tty_close() using hu->proto->close(),
   which sets ll->priv to NULL
4) hci_uart_write_work() calls hci_uart_dequeue() , which calls
   ll_dequeue() , which accesses hu->priv->txq using skb_dequeue() .
   In fact, skb_dequeue() locks the spinlock lock in hu->priv->txq->lock
   first , which is located at offset 0x18 from start of hu->priv
   (struct ll_struct) .

NOTE: The system I use is so far on 4.7, but I will be rebasing this forward.
      I would like to check with you whether I could be hitting something or
      whether my hypothesis is completely wrong and there's possibly some
      other problem. Thanks!

Signed-off-by: Marek Vasut <marex@denx.de>
---
Unable to handle kernel NULL pointer dereference at virtual address 00000018
pgd = c0004000
[00000018] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in: evbug wl12xx wlcore_sdio wlcore
CPU: 0 PID: 21 Comm: kworker/0:1 Not tainted 4.7.10-00952-gb49feff6-dirty #64
Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events hci_uart_write_work
task: da4d2a00 ti: da59e000 task.ti: da59e000
PC is at _raw_spin_lock_irqsave+0x1c/0x5c
LR is at skb_dequeue+0x1c/0x70
pc : [<c08e045c>]    lr : [<c070a944>]    psr: 600d0093
sp : da59fe70  ip : da59fe80  fp : da59fe7c
r10: 00000000  r9 : d8decc5c  r8 : 00000005
r7 : d8c54000  r6 : 00000018  r5 : 0000000c  r4 : 00000000
r3 : c06674bc  r2 : 00000018  r1 : d8dec8bc  r0 : 600d0013
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 98e2c04a  DAC: 00000051
Process kworker/0:1 (pid: 21, stack limit = 0xda59e210)
Stack: (0xda59fe70 to 0xda5a0000)
fe60:                                     da59fe9c da59fe80 c070a944 c08e044c
fe80: 00000000 d8dec8a0 d8decb00 d8c54000 da59feac da59fea0 c06674d4 c070a934
fea0: da59fee4 da59feb0 c0666acc c06674c8 d8dec880 d8dec8bc da59fee4 d8dec8a0
fec0: da561e00 dabad700 00000000 dabb0a00 00000000 da561e00 da59ff1c da59fee8
fee0: c013a208 c06669d8 c0d02100 da59e000 dabad700 da561e18 00000008 dabad718
ff00: c0d02100 da59e000 dabad700 da561e00 da59ff5c da59ff20 c013af04 c013a028
ff20: 00000000 da561e00 c013aec4 00000000 00000000 00000000 da563300 da561e00
ff40: c013aec4 00000000 00000000 00000000 da59ffac da59ff60 c01402d8 c013aed0
ff60: 00005423 00000000 c0140200 da561e00 00000000 00000000 da59ff78 da59ff78
ff80: 00000000 00000000 da59ff88 da59ff88 da563300 c0140200 00000000 00000000
ffa0: 00000000 da59ffb0 c0107b98 c014020c 00000000 00000000 00000000 00000000
ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff 00000000
Backtrace:
[<c08e0440>] (_raw_spin_lock_irqsave) from [<c070a944>] (skb_dequeue+0x1c/0x70)
[<c070a928>] (skb_dequeue) from [<c06674d4>] (ll_dequeue+0x18/0x1c)
 r7:d8c54000 r6:d8decb00 r5:d8dec8a0 r4:00000000
[<c06674bc>] (ll_dequeue) from [<c0666acc>] (hci_uart_write_work+0x100/0x130)
[<c06669cc>] (hci_uart_write_work) from [<c013a208>] (process_one_work+0x1ec/0x418)
 r10:da561e00 r9:00000000 r8:dabb0a00 r7:00000000 r6:dabad700 r5:da561e00
 r4:d8dec8a0
[<c013a01c>] (process_one_work) from [<c013af04>] (worker_thread+0x40/0x5bc)
 r10:da561e00 r9:dabad700 r8:da59e000 r7:c0d02100 r6:dabad718 r5:00000008
 r4:da561e18
[<c013aec4>] (worker_thread) from [<c01402d8>] (kthread+0xd8/0xf8)
 r10:00000000 r9:00000000 r8:00000000 r7:c013aec4 r6:da561e00 r5:da563300
 r4:00000000
[<c0140200>] (kthread) from [<c0107b98>] (ret_from_fork+0x14/0x3c)
 r7:00000000 r6:00000000 r5:c0140200 r4:da563300
Code: e1a02000 e10f0000 f10c0080 f5d2f000 (e1923f9f)
---[ end trace fe4ab10f7f9fd772 ]---
---
 drivers/bluetooth/hci_ldisc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 49b3e1e..f02f406 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -253,7 +253,8 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hu->proto->enqueue(hu, skb);
 
-	hci_uart_tx_wakeup(hu);
+	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
+		hci_uart_tx_wakeup(hu);
 
 	return 0;
 }
@@ -477,6 +478,7 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 {
 	struct hci_uart *hu = tty->disc_data;
 	struct hci_dev *hdev;
+	unsigned long flags;
 
 	BT_DBG("tty %p", tty);
 
@@ -490,9 +492,11 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (hdev)
 		hci_uart_close(hdev);
 
+	flags = test_and_clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+
 	cancel_work_sync(&hu->write_work);
 
-	if (test_and_clear_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+	if (flags) {
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
-- 
2.10.2