Hi Andrei,
On Mon, Jan 22, 2024 at 7:18 AM Andrei Volkov
<andrey.volkov@xxxxxxxxxxxxxxxxx> wrote:
>
> Hello,
>
> Lately we've bumped with regression introduced by commit:
>
> c7f59461f5a78 ("Bluetooth: Fix a refcnt underflow problem for
> hci_conn", 2023-10-04)
>
> The regression related with adding "hci_conn_ssp_enabled()" check in
> "hci_io_capa_request_evt()" handler, and broke pairing process initiated
> by the external device.
>
> Precisely, some ext. devices, like any phone equipped with Android ver <
> 14 (we have not latest one, so we didn't check), always send "IO
> Capability Request" before "Read Remote Extended Features" command, as
> consequence the flag "HCI_CONN_SSP_ENABLED" not yet activated at the
> time of "hci_io_capa_request_evt()" execution and
> "hci_conn_ssp_enabled()" always returns false in time of the pairing.
>
> As a result, pairing always fails. The quick and dirty fix is revert the
> ssp check introduced in the subj. commit, like below:
>
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5329,7 +5329,7 @@ static void hci_io_capa_request_evt(struct hci_dev
> *hdev, void *data,
> hci_dev_lock(hdev);
>
> conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
> - if (!conn || !hci_conn_ssp_enabled(conn))
> + if (!conn)
> goto unlock;
>
> hci_conn_hold(conn);
>
>
> However, a more thorough and correct fix requires discussion and
> testing. Therefore, I would like to get any comments/suggestion from the
> community before doing this.
I think we need to do something like this, so we consider only the
local SSP flag when evaluating if we should proceed to respond or just
drop:
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6130c969f361..a15924db83d9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5327,9 +5327,12 @@ static void hci_io_capa_request_evt(struct
hci_dev *hdev, void *data,
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
- if (!conn || !hci_conn_ssp_enabled(conn))
+ if (!conn || !hci_dev_test_flag(hdev, HCI_SSP_ENABLED))
goto unlock;
+ /* Assume remote supports SSP since it has triggered this event */
+ set_bit(HCI_CONN_SSP_ENABLED, &conn->flags);
+
hci_conn_hold(conn);
if (!hci_dev_test_flag(hdev, HCI_MGMT))
> Regards
> Andrey VOLKOV
>
>
--
Luiz Augusto von Dentz
