netdev - Re: [PATCH RFC] bluetooth: add support for some old headsets

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] [day] [month] [year] [list]

Message-ID: <d9ebfb9e-5abb-0898-b5e1-0ff55a07d79d@omprussia.ru>
Date:   Tue, 28 Jul 2020 13:27:54 +0000
From:   Ildar Kamaletdinov <i.kamaletdinov@...russia.ru>
To:     Marcel Holtmann <marcel@...tmann.org>,
        Sergey Shtylyov <s.shtylyov@...russia.ru>
CC:     Johan Hedberg <johan.hedberg@...il.com>,
        Bluetooth Kernel Mailing List 
        <linux-bluetooth@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC] bluetooth: add support for some old headsets

Hello everyone,

Let me add a little bit more details related to this patch.

According to Bluetooth Core Specification Vol 2 part F page 776 and  Bluetooth Core Specification Vol4 Part E page 1978 after `HCI_Setup_Synchronous_Connection()` was sent `HCI_Command_Status()` must be received from BDR/EDR controller.

According to Bluetooth Core Specification Vol 2 part F page 364 HCI_Command_Status description:

"Some HCI commands may generate errors that need to be reported to the Host, but there is insufficient information to determine how the command would normally be processed. In this case, two events can be used to indicate this to the Host, the HCI_Command_Complete event and HCI_Command_Status events. Which of the two events is used is implementation-dependent."

Mediatek's implementation reports error `Unsupported LMP feature` in `HCI_Command_Status()` event (not in `HCI_Command_Complete()` event as in other implementations).
So that behavior is a little bit odd but don't break compatibility with Bluetooth Core Specification. Actually Mediatek's BDR/EDR controller reports error without trying to actually setup connection with headset.

But according to Hands-Free profile specification 1.8 p. 113 Synchronous Connection Interoperability Requirements Bluetooth Host MUST try all features T1->T2->S1->S2->D0 or D1 before considering connection as `failed`. And it is true if error is reported in `HCI_Command_Complete()` event.
Eventually If error is reported in `HCI_Command_Status()` event connection considered as 'failed' just after first error which breaks compatibility with HFP profile specification v1.8 in Linux Kernel. That leads to problems when SCo/eSCO connection could not be established when using Mediatek's BDR/EDR controllers.

So that patch should add support for correct SCO/eSCO connection behavior when work with at least Mediatek controllers. (a little bit peculiar behavior but compatible with Bluetooth Core spec).

If any objections or questions please fill free to contact me or Sergey anytime.
> Hi Sergey,
>
>>>>>>> The MediaTek Bluetooth platform (MT6630 etc.) has a peculiar implementation
>>>>>>> for the eSCO/SCO connection via BT/EDR: the host controller returns error
>>>>>>> code 0x20 (LMP feature not supported) for HCI_Setup_Synchronous_Connection
>>>>>>> (0x0028) command without actually trying to setup connection with a remote
>>>>>>> device in case such device (like Digma BT-14 headset) didn't advertise its
>>>>>>> supported features.  Even though this doesn't break compatibility with the
>>>>>>> Bluetooth standard it breaks the compatibility with the Hands-Free Profile
>>>>>>> (HFP).
>>>>>>>
>>>>>>> This patch returns the compatibility with the HFP profile and actually
>>>>>>> tries to check all available connection parameters despite of the specific
>>>>>>> MediaTek implementation. Without it one was unable to establish eSCO/SCO
>>>>>>> connection with some headsets.
>> [...]
>>>>>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@...russia.ru>
>>>>>>>
>>>>>>> ---
>>>>>>> This patch is against the 'bluetooth-next.git' repo.
>>>>>>>
>>>>>>> net/bluetooth/hci_event.c |    8 ++++++++
>>>>>>> 1 file changed, 8 insertions(+)
>>>>>>>
>>>>>>> Index: bluetooth-next/net/bluetooth/hci_event.c
>>>>>>> ===================================================================
>>>>>>> --- bluetooth-next.orig/net/bluetooth/hci_event.c
>>>>>>> +++ bluetooth-next/net/bluetooth/hci_event.c
>>>>>>> @@ -2187,6 +2187,13 @@ static void hci_cs_setup_sync_conn(struc
>>>>>>> 	if (acl) {
>>>>>>> 		sco = acl->link;
>>>>>>> 		if (sco) {
>>>>>>> +			if (status == 0x20 && /* Unsupported LMP Parameter value */
>>>>>>> +			    sco->out) {
>>>    Actually, I was expecting that you'd tell me to create a HCI quirk for this situation.
>>> I have a patch doing that but I haven't been able to locate the driver in which to set this
>>> quirk flag...
>>   And that's no wonder! The BT driver that needs this patch is out-of-tree (and not even open
>> source, it seems) as we have finally ascertained with Ildar... Is there any interest in the
>> "preparatory" patch that lowers the indentation levels in hci_cs_setup_sync_conn()?
> how is it possible that there is an out-of-tree Bluetooth driver. Seems odd. Maybe want to submit that upstream first.
>
> Regards
>
> Marcel
>