Re: [PATCH 6.12 106/337] Bluetooth: hci_sync: remove unnecessary hci_conn_get in create_conn_sync

Hi Greg/Sasha

On 07/08/26 8:05 pm, Greg Kroah-Hartman wrote:
> 6.12-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Pauli Virtanen <[email protected]>
> 
> [ Upstream commit c0a9dcd2be398eee505d4b254ec3a845aa8ab189 ]
> 
> hci_conn_get() without already held reference is data race against
> concurrent deletion.
> 
> In previous patches, the refcount has been changed to be taken before
> starting the hci_sync task, so remove these extra get() + put() as they
> are not needed.
> 


I have run an AI assisted backport review and it spotted an issue; I 
checked 6.12.y tree and the prerequisite change is missing.

Upstream first changed the callers to queue a held reference:

err = hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync,
			      hci_conn_get(conn),
			      hci_acl_create_conn_sync_complete);
if (err)
	hci_conn_put(conn);


The posted 6.12.y state still queues the raw pointer:

return hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync, conn,
			       NULL);


This backport nevertheless removes the worker-side get/put pairs. That 
leaves the queued work without a reference if connection deletion races 
it. I think 6.12.y misses commit: 2f5d635ad5906 ("Bluetooth:
hci_sync: hold conn in hci_connect_acl/le_sync() callbacks"); I think we 
have to either queue this prerequisite or drop this, thoughts ?

Also this is part of the series: 
https://lore.kernel.org/all/178517880663.1372023.9123649060094265539.git-patchwork-notify@kernel.org/

thanks,
Harshit

> Fixes: 12917f591cea ("Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()")
> Signed-off-by: Pauli Virtanen <[email protected]>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
>   net/bluetooth/hci_sync.c | 13 -------------
>   1 file changed, 13 deletions(-)
> 
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 8c48d459a5415..03bbf4c1d6e5a 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -6577,11 +6577,6 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
>   
>   	bt_dev_dbg(hdev, "conn %p", conn);
>   
> -	/* Hold a reference so conn stays valid for the HCI_CONN_CREATE
> -	 * clear_bit() at done.
> -	 */
> -	hci_conn_get(conn);
> -
>   	clear_bit(HCI_CONN_SCANNING, &conn->flags);
>   	conn->state = BT_CONNECT;
>   
> @@ -6596,7 +6591,6 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
>   			conn->state = BT_OPEN;
>   			hci_abort_conn_sync(hdev, conn,