netdev - Re: [PATCH v3 0/4] Bluetooth: Improve retrying of connection attempts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <efcc7b97-6bfc-4e5d-8e73-78f2b190fa02@v0yd.nl>
Date: Tue, 9 Jan 2024 22:57:47 +0100
From: Jonas Dreßler <verdre@...d.nl>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: Marcel Holtmann <marcel@...tmann.org>,
 Johan Hedberg <johan.hedberg@...il.com>, linux-bluetooth@...r.kernel.org,
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org, verdre@...d.nl
Subject: Re: [PATCH v3 0/4] Bluetooth: Improve retrying of connection attempts

Hi Luiz,

On 1/9/24 18:53, Luiz Augusto von Dentz wrote:
> Hi Jonas,
> 
> On Mon, Jan 8, 2024 at 5:46 PM Jonas Dreßler <verdre@...d.nl> wrote:
>>
>> Since commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect
>> requests"), the kernel supports trying to connect again in case the
>> bluetooth card is busy and fails to connect.
>>
>> The logic that should handle this became a bit spotty over time, and also
>> cards these days appear to fail with more errors than just "Command
>> Disallowed".
>>
>> This series refactores the handling of concurrent connection requests
>> by serializing all "Create Connection" commands for ACL connections
>> similar to how we do it for LE connections.
>>
>> ---
>>
>> v1: https://lore.kernel.org/linux-bluetooth/20240102185933.64179-1-verdre@v0yd.nl/
>> v2: https://lore.kernel.org/linux-bluetooth/20240108183938.468426-1-verdre@v0yd.nl/
>> v3:
>>    - Move the new sync function to hci_sync.c as requested by review
>>    - Abort connection on failure using hci_abort_conn_sync() instead of
>>      hci_abort_conn()
>>    - Make the last commit message a bit more precise regarding the meaning
>>      of BT_CONNECT2 state
>>
>> Jonas Dreßler (4):
>>    Bluetooth: Remove superfluous call to hci_conn_check_pending()
>>    Bluetooth: hci_event: Use HCI error defines instead of magic values
>>    Bluetooth: hci_conn: Only do ACL connections sequentially
>>    Bluetooth: Remove pending ACL connection attempts
>>
>>   include/net/bluetooth/hci.h      |  3 ++
>>   include/net/bluetooth/hci_core.h |  1 -
>>   include/net/bluetooth/hci_sync.h |  3 ++
>>   net/bluetooth/hci_conn.c         | 83 +++-----------------------------
>>   net/bluetooth/hci_event.c        | 29 +++--------
>>   net/bluetooth/hci_sync.c         | 72 +++++++++++++++++++++++++++
>>   6 files changed, 93 insertions(+), 98 deletions(-)
>>
>> --
>> 2.43.0
> 
> After rebasing and fixing a little bit here and there, see v4, looks
> like this changes is affecting the following mgmt-tester -s "Pair
> Device - Power off 1":
> 
> Pair Device - Power off 1 - init
>    Read Version callback
>      Status: Success (0x00)
>      Version 1.22
>    Read Commands callback
>      Status: Success (0x00)
>    Read Index List callback
>      Status: Success (0x00)
>    Index Added callback
>      Index: 0x0000
>    Enable management Mesh interface
>    Enabling Mesh feature
>    Read Info callback
>      Status: Success (0x00)
>      Address: 00:AA:01:00:00:00
>      Version: 0x09
>      Manufacturer: 0x05f1
>      Supported settings: 0x0001bfff
>      Current settings: 0x00000080
>      Class: 0x000000
>      Name:
>      Short name:
>    Mesh feature is enabled
> Pair Device - Power off 1 - setup
>    Setup sending Set Bondable (0x0009)
>    Setup sending Set Powered (0x0005)
>    Initial settings completed
>    Test setup condition added, total 1
>    Client set connectable: Success (0x00)
>    Test setup condition complete, 0 left
> Pair Device - Power off 1 - setup complete
> Pair Device - Power off 1 - run
>    Sending Pair Device (0x0019)
> Bluetooth: hci0: command 0x0405 tx timeout
> Bluetooth: hci0: command 0x0408 tx timeout
>    Test condition added, total 1
> Pair Device - Power off 1 - test timed out
>    Pair Device (0x0019): Disconnected (0x0e)
> Pair Device - Power off 1 - test not run
> Pair Device - Power off 1 - teardown
> Pair Device - Power off 1 - teardown
>    Index Removed callback
>      Index: 0x0000
> Pair Device - Power off 1 - teardown complete
> Pair Device - Power off 1 - done
> 

Thanks for landing the first two commits!

I think this is actually the same issue causing the test failure
as in the other issue I had:
https://lore.kernel.org/linux-bluetooth/7cee4e74-3a0c-4b7c-9984-696e646160f8@v0yd.nl/

It seems that the emulator is unable to reply to HCI commands sent
from the hci_sync machinery, possibly because that is sending things
on a separate thread?

Cheers,
Jonas