Received-SPF: pass (google.com: domain of linux-kernel+bounces-141298-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <cfe268c1-9395-da7f-2db5-46152e348274@basealt.ru>
Date: Thu, 11 Apr 2024 19:14:34 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH net] Bluetooth: hci_event: fix possible multiple drops by
 marked conn->state after hci_disconnect()
Content-Language: en-US
To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com,
 linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
References: <20240411151929.403263-1-kovalev@altlinux.org>
From: kovalev@altlinux.org
In-Reply-To: <20240411151929.403263-1-kovalev@altlinux.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

11.04.2024 18:19, kovalev@altlinux.org wrote:
> From: Vasiliy Kovalev <kovalev@altlinux.org>
> 
> 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/f8bb62a7-5845-53ed-7fbe-c0557c2745f2@basealt.ru/#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: stable@vger.kernel.org
> Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> ---
>   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