Re: [BUG] BLE device unpairing triggers kernel panic

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Luiz,

On 14.05.22 01:52, Luiz Augusto von Dentz wrote:
> On Fri, May 13, 2022 at 1:14 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:

Thanks for the quick reply.

>> Hi Ahmad,
>>
>> On Fri, May 13, 2022 at 7:10 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>
>>> Hello,
>>>
>>> On Linux v5.18-rc5, I can reliably crash the kernel on the second (un)pairing
>>> with a customer's BLE device. I have bisected the issue and found two commits:
>>>
>>> - Commit 6cd29ec6ae5e ("Bluetooth: hci_sync: Wait for proper events when
>>>   connecting LE") causes previously working pairing to time out, presumably
>>>   because it keeps waiting for the wrong event.
>>
>> Can you describe in more details what is the second pairing, are you
>> pairing 2 devices concurrently? I recall someone for nxp having
>> similar problem, at least the traces look pretty similar, the problem
>> seems to be the expected event don't match the event the controller
>> send, in this case hci_le_enh_conn_complete_evt, so hci_event process
>> it and frees the hci_conn instead of first running the callback.

It's the same device. I set the host to pairable, then have the device
pair with the host. Then I unpair on the device and then redo the
same operation again. First one works. Second one fails triggering
the crash.

> Looks like my memory failed me on this one, the sync callback is run
> last so we shouldn't cleanup the hci_conn at that point, perhaps
> something like the following should fix the crash:
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 0270e597c285..c1634af670b8 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5632,10 +5632,8 @@ static void le_conn_complete_evt(struct hci_dev
> *hdev, u8 status,
>                 status = HCI_ERROR_INVALID_PARAMETERS;
>         }
> 
> -       if (status) {
> -               hci_conn_failed(conn, status);
> +       if (status)
>                 goto unlock;
> -       }

Yes, this fixes the crash for me. Can you send a patch to that effect?
Feel free to add:

  Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

>         if (conn->dst_type == ADDR_LE_DEV_PUBLIC)
>                 addr_type = BDADDR_LE_PUBLIC;
> 
>>> - Commit a56a1138cbd8 ("Bluetooth: hci_sync: Fix not using conn_timeout")
>>>   fixes, despite the title, what event is waited on. First Pairing works now,
>>>   but the second pairing times out and crashes the kernel:
>>>
>>>   [   84.191684] Bluetooth: hci0: Opcode 0x200d failed: -110
>>>   [   84.230478] Bluetooth: hci0: request failed to create LE connection: err -110
>>>   [   84.237690] Unable to handle kernel read from unreadable memory at virtual address 0000000000000ca8
> 
> That said the error -110 mean -ETIMEDOUT

Yes, this issue remains still. I feel better about my revert
knowing that the crash is fixed, but I'd like this regression
here fixed upstream as well. I'll try to collect some more
information and report back.

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |