Received-SPF: pass (google.com: domain of linux-kernel+bounces-159768-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 26 Apr 2024 12:16:45 +0300
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Sungwoo Kim <iam@sung-woo.kim>
Cc: daveti@purdue.edu, Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: L2CAP: Fix slab-use-after-free in
 l2cap_send_cmd
Message-ID: <cff764c2-a3d1-4a12-9260-54122e7a1fef@moroto.mountain>
References: <20240426072006.358802-1-iam@sung-woo.kim>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240426072006.358802-1-iam@sung-woo.kim>

On Fri, Apr 26, 2024 at 03:20:05AM -0400, Sungwoo Kim wrote:
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 84fc70862..a8f414ab8 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3953,6 +3953,9 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>  	if (!chan)
>  		goto response;
>  
> +	l2cap_chan_hold(chan);
> +	l2cap_chan_lock(chan);
> +
>  	/* For certain devices (ex: HID mouse), support for authentication,
>  	 * pairing and bonding is optional. For such devices, inorder to avoid
>  	 * the ACL alive for too long after L2CAP disconnection, reset the ACL
> @@ -4041,6 +4044,11 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>  		chan->num_conf_req++;
>  	}
>  
> +	if (chan) {
> +		l2cap_chan_unlock(chan);
> +		l2cap_chan_put(chan);
> +	}
> +
>  	return chan;
        ^^^^^^^^^^^^
This doesn't fix the bug because we're returning chan.

As soon as you call l2cap_chan_put() then chan will be freed by in the
other thread which is doing l2cap_conn_del() resulting in a use after
free in the caller.

regards,
dan carpenter