Re: [PATCH] fix: net/bluetooth: iso_conn_del: extra iso_conn_put on iso_sock_hold failure path

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Wen Tao,


Thank you for your patch. Some formalities:

The prefixes are well defined, and `fix:` will make the CI fail. Also, 
it’d be great if you removed the tripple space in the summary/title, and 
made it a statement by adding a verb in imperative mood. Maybe:

Bluetooth: Fix extra iso_conn_put on iso_sock_hold failure path

Am 26.06.26 um 13:53 schrieb WenTao Liang:
> In iso_conn_del(), iso_conn_hold_unless_zero() acquires a temporary
>    reference which is correctly balanced by iso_conn_put() at line 279. When
>    iso_sock_hold() returns NULL (sk == NULL), an additional
>    iso_conn_put(conn) is called, dropping hcon's reference to conn too
>    early. The caller (e.g., hci_conn_del) will later also iso_conn_put(),
>    causing a double-free or use-after-free.

It’s uncommon to indent any lines of a paragraph. I recommend to remove 
it, and then each line also fits in 75 characters.

> Remove the extra iso_conn_put(conn) on the sk == NULL path.

Out of curiosity: Do you have a reproducer?

> Cc: stable@vger.kernel.org
> Fixes: dc26097bdb86 ("Bluetooth: ISO: Use kref to track lifetime of iso_conn")
> Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
> ---
>   net/bluetooth/iso.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 3abd8111dda8..99755671e469 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -278,10 +278,8 @@ static void iso_conn_del(struct hci_conn *hcon, int err)
>   	iso_conn_unlock(conn);
>   	iso_conn_put(conn);
>   
> -	if (!sk) {
> -		iso_conn_put(conn);
> +	if (!sk)
>   		return;
> -	}
>   
>   	lock_sock(sk);
>   	iso_sock_clear_timer(sk);

gemini/gemini-3.1-pro-preview has two comments [1].


Kind regards,

Paul


[1]: 
https://sashiko.dev/#/patchset/20260626115312.33528-1-vulab%40iscas.ac.cn