Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5aca90c4-8436-40d1-87d5-3406fdb26c47@buaa.edu.cn>
Date:   Wed, 9 Aug 2023 23:10:27 +0800
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [BUG]Bluetooth: possible semantic bug when the status field of
 the HCI_Connection_Complete packet set to non-zero
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     marcel@holtmann.org, johan.hedberg@gmail.com, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        baijiaju1990@gmail.com, sy2239101@buaa.edu.cn,
        linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org
References: <CABBYNZLQo-WhM9jDJbk_zXu-ETdv8QkJ5UG9d+nWDBEA66Y+VQ@mail.gmail.com>
From:   Xin-Yu Liu <LXYbhu@buaa.edu.cn>
In-Reply-To: <CABBYNZLQo-WhM9jDJbk_zXu-ETdv8QkJ5UG9d+nWDBEA66Y+VQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

Thanks for your reply.

After carefully considering your feedback, we now realize that the scenario we were assuming is indeed quite exceptional. It has become clear that there is no necessity for any supplementary mechanisms to reset the Bluetooth system. In our forthcoming work, we plan to engage in an in-depth exploration of the spec and Linux Bluetooth source code, with the aim of deriving more valuable insights and outcomes.

Once again, thank you for your active engagement and for taking the time to address our queries. Your assistance has been instrumental in guiding our approach.

Wishing you all the best and looking forward to future interactions.

Warm regards,
Xin-Yu Liu

2023/8/8 1:48, Luiz Augusto von Dentz :
> Hi,
>
> On Mon, Aug 7, 2023 at 8:17 AM Xin-Yu Liu <LXYbhu@buaa.edu.cn> wrote:
>> Hello,
>>
>> Thanks for your reply.
>>
>> I apologize for my previous unclear statement, which may have misled you.
>>
>> Let me rephrase our question:
>>
>> When a Bluetooth device initiates a connection to another device, its host sends an HCI_Create_Connection command (OGF: 0x01, OCF: 0x0005) to the controller. Once the connection is established, the controller sends an HCI_Connection_Complete event (Event Code: 0x03) back to the host. If a valid HCI_Connection_Complete event has its parameter "Status" altered (with all other parameters unchanged), changing 0x00 to any value between 0x01 and 0xFF for example, the host will considerd that the connection fails to complete.
>>
>> In reality, if the HCI_Connection_Complete event's parameter "Connection_Handle" is valid and unaltered, it means the handle resource exists and has not been released. The observations we made support this statement:
>>
> Well according to the spec we can only assume the handle is valid if
> the status is set to 0x00, so I am not really sure how we can possibly
> check if the handle is valid if the status indicates a connection
> failure?
>
>> (a) When the tampered HCI_Connection_Complete event with altered "Status" is sent to the host, if we attempt to reconnect to the same device by sending another HCI_Create_Connection command, the controller will send an HCI_Command_Status event (Event Code: 0x0F) to the host, with the "Status" parameter set to 0x0B, indicating "CONNECTION ALREADY EXISTS" and leading to the connection failure.
>>
>> (b) When the tampered HCI_Connection_Complete event is sent to the host, if we manually send an HCI_Disconnect command, with the "Connection_Handle" parameter set to the same value as the previous HCI_Connection_Complete event's "Connection_Handle," and the "Reason" parameter set to 0x15, indicating "REMOTE DEVICE TERMINATED CONNECTION DUE TO POWER OFF," we receive a proper response, signifying that the Connection_Handle is valid and exists. Additionally, the issue described in (a) disappears.
> Just read again the sentence above: 'TERMINATED CONNECTION', it can't
> possible mean the handle is valid and exists, I'm afraid you are
> arguing based on a controller implementation that doesn't comply with
> the spec text above, it shall either disconnect the link so we
> invalidate the handle on the host, then later we can reconnect, or
> indicate the status is 0x00.
>
>> Well we can't do much about the dangling connection if we don't know
>> its handle to be able to disconnect since there is no command to
>> disconnect by address if that is what you were expecting us to do, so
>> the bottom line seems to be that sending 0x0b to the controller is
>> useless since we can't do anything about at the host, well other than
>> reset but would likely affect other functionality as well.
>>
>> With knowledge of the handle, we think we can manually send an HCI_Disconnect command to deal with the dangling connection, just as we mentioned in (b).
> Assuming the handle is valid on status != 0x00 would probably not work
> with most controllers following the spec to the letter, in which case
> the HCI_Disconnect would fail and in the meantime we have an hci_conn
> with invalid state, so I don't think it is worth going sideways just
> to get it working under special circumstances, where this special
> circumstances might be a bug in the way status is used.
>
>> We believe that, in the situation we mentioned, the handle is valid but is rendered useless. Implementing an automated mechanism to handle the release of the handle (e.g., by sending an HCI_Disconnect command) might be a better choice.
> Sorry but I have to disagree, in that case HCI_Disconnect would need
> to be sent every time, which can also fail if the link-layer had
> terminated the connection as indicated in the status.
>
>> Best wishes,
>> Xin-Yu Liu
>>
>> 2023/8/5 13:09, Luiz Augusto von Dentz :
>>
>> Hi,
>>
>> On Fri, Aug 4, 2023 at 9:35 PM Xinyu Liu <LXYbhu@buaa.edu.cn> wrote:
>>
>> Hello,
>>
>> Our fuzzing tool finds a possible semantic bug in the Bluetooth system in Linux 6.2:
>>
>> During the connection process, the host server needs to receive the HCI_Connection_Complete packet from the hardware controller. In normal cases, the status field of this packet is zero, which means that the connection is successfully completed:
>>
>> However, in our testing, when the status field was set to non-zero, 47 for instance, the Bluetooth connection failed. After that, when we attempt to reestablish a Bluetooth connection, the connection always fails. Upon analyzing the event packets sent from the controller to the host server, we observed that the Status field of the HCI_Command_Status packet becomes 0B, indicating that the controller believes the connection already exists. This situation has been causing the connection failure persistently:
>>
>> That seems like a link-layer issue, the controller is saying the
>> connection had failed, and 0x0b also doesn't help either except if you
>> are saying that the other parameters are actually valid (e.g. handle),
>> that said the spec seems pretty clear about status other than 0x00
>> means the connection had failed:
>>
>> BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
>> page 2170
>>
>> 0x01 to 0xFF Connection failed to Complete. See [Vol 1] Part F,
>> Controller Error Codes
>> for a list of error codes and descriptions.
>>
>> In our understanding, it would be more preferable if a single failed Bluetooth connection does not result in subsequent connections also failing. We believe that having some mechanism to facilitate Bluetooth's recovery and restoration to normal functionality could be considered as a potentially better option.
>>
>> We are not sure whether this is a semantic bug or implementation feature in the Linux kernel. Any feedback would be appreciated, thanks!
>>
>> Well we can't do much about the dangling connection if we don't know
>> its handle to be able to disconnect since there is no command to
>> disconnect by address if that is what you were expecting us to do, so
>> the bottom line seems to be that sending 0x0b to the controller is
>> useless since we can't do anything about at the host, well other than
>> reset but would likely affect other functionality as well.
>>
>>
>