On 12/10/2016 02:36 AM, Marek Vasut wrote:
> 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@xxxxxxx>
Bump ?
> ---
> 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);
>
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
