2024-04-11 17:43:11

by Vasiliy Kovalev

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: hci_event: fix possible multiple drops by marked conn->state after hci_disconnect()

11.04.2024 18:19, [email protected] wrote:
> From: Vasiliy Kovalev <[email protected]>
>
> When returning from the hci_disconnect() function, the conn->state
> continues to be set to BT_CONNECTED and hci_conn_drop() is executed,
> which decrements the conn->refcnt.

Syzkaller C reproducer:
https://lore.kernel.org/all/[email protected]/#t

During debugging, the value conn->refcnt goes down to -1000 and less.

> Syzkaller has generated a reproducer that results in multiple calls to
> hci_encrypt_change_evt() of the same conn object.
> --
> hci_encrypt_change_evt(){
> // conn->state == BT_CONNECTED
> hci_disconnect(){
> hci_abort_conn();
> }
> hci_conn_drop();
> // conn->state == BT_CONNECTED
> }
> --
> This behavior can cause the conn->refcnt to go far into negative values
> and cause problems. To get around this, you need to change the conn->state,
> namely to BT_DISCONN, as it was before. > Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for
aborting connections")
> Cc: [email protected]
> Signed-off-by: Vasiliy Kovalev <[email protected]>
> ---
> net/bluetooth/hci_event.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 64477e1bde7cec..e0477021183f9b 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2989,6 +2989,7 @@ static void hci_cs_le_start_enc(struct hci_dev *hdev, u8 status)
>
> hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
> hci_conn_drop(conn);
> + conn->state = BT_DISCONN;
>
> unlock:
> hci_dev_unlock(hdev);
> @@ -3654,6 +3655,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, void *data,
> hci_encrypt_cfm(conn, ev->status);
> hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
> hci_conn_drop(conn);
> + conn->state = BT_DISCONN;
> goto unlock;
> }
>
> @@ -5248,6 +5250,7 @@ static void hci_key_refresh_complete_evt(struct hci_dev *hdev, void *data,
> if (ev->status && conn->state == BT_CONNECTED) {
> hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
> hci_conn_drop(conn);
> + conn->state = BT_DISCONN;
> goto unlock;
> }
>

--
Regards,
Vasiliy Kovalev