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

Message-Id: <848144D3-85F9-47F8-8CDA-02457FA7530F@holtmann.org>
Date:   Fri, 17 Jul 2020 08:59:44 +0200
From:   Marcel Holtmann <marcel@...tmann.org>
To:     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
Subject: Re: [PATCH RFC] bluetooth: add support for some old headsets

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.
>> 
>> please include the parts of btmon output that show this issue.
> 
>   Funny, I had removed that part from the original patch. Here's that log:
> 
> < HCI Command: Setup Synchronous Connection (0x01|0x0028) plen 17                                  #1 [hci0] 6.705320
>        Handle: 50
>        Transmit bandwidth: 8000
>        Receive bandwidth: 8000
>        Max latency: 10
>        Setting: 0x0060
>          Input Coding: Linear
>          Input Data Format: 2's complement
>          Input Sample Size: 16-bit
>            of bits padding at MSB: 0
>          Air Coding Format: CVSD
>        Retransmission effort: Optimize for power consumption (0x01)
>        Packet type: 0x0380
>          3-EV3 may not be used
>          2-EV5 may not be used
>          3-EV5 may not be used
>> HCI Event: Command Status (0x0f) plen 4                                                          #2 [hci0] 6.719598
>      Setup Synchronous Connection (0x01|0x0028) ncmd 1
>        Status: Unsupported LMP Parameter Value / Unsupported LL Parameter Value (0x20)

I double check with the specification and it is not precise that errors should be reported via sync conn complete events. My assumption would be that your headset only supports SCO and thus the controller realizes that eSCO request can not be completed anyway. So the controller opts for quickest path to get out of this.

> 
>>> Based on the patch by Ildar Kamaletdinov <i.kamaletdinov@...russia.ru>.
>>> 
>>> 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) {
>>> +				sco->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
>>> +						(hdev->esco_type & EDR_ESCO_MASK);
>>> +				if (hci_setup_sync(sco, sco->link->handle))
>>> +					goto unlock;
>>> +			}
>>> 			sco->state = BT_CLOSED;
>> 
>> since this is the command status event, I doubt that sco->out check is needed.
> 
>   Can't comment oin this, my BT fu is too weak... 

It is the case. Command status is only local to command we issued and thus in this case it is the connection creation attempt from our side. Meaning it is always outgoing.

Regards

Marcel