Forwarded: Re: [RESEND] Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user

[syzbot <syzbot+14b6d57fb728e27ce23c@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: Re: [RESEND] Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user
Author: pav@iki.fi

la, 2026-03-07 kello 11:45 +0200, Pauli Virtanen kirjoitti:
> la, 2026-03-07 kello 10:59 +0200, Pauli Virtanen kirjoitti:
> > pe, 2026-03-06 kello 16:04 -0500, Luiz Augusto von Dentz kirjoitti:
> > > From: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
> > > 
> > > After commit ab4eedb790ca ("Bluetooth: L2CAP: Fix corrupted list in
> > > hci_chan_del"), l2cap_conn_del() uses conn->lock to protect access to
> > > conn->users and conn->hchan. However, l2cap_register_user() and
> > > l2cap_unregister_user() still use hci_dev_lock(), creating a race
> > > condition where these functions can access conn->users and conn->hchan
> > > concurrently with l2cap_conn_del().
> > 
> > AFAIK the above text from the original submitter is a bit inaccurate,
> > as l2cap_conn_del() is called with hdev lock held, so conn->users/hchan
> > should be safe.
> > 
> > However, using conn->mutex should fix the use-after-free in
> > 
> > 	conn->hcon->hdev
> > 	hci_dev_lock(hdev);
> > 	hci_dev_unlock(hdev);
> > 
> > by making l2cap_unregister_user() safe to call after the hcon/hdev are
> > no longer alive.
> > 
> > The change looks OK to me, but probably worth to double check with
> > syzbot it fixes the original issue
> 
> syzbot seems to have hit some internal error, another try on upstream
> branch instead
> 

No luck, test the patch setting session->conn = NULL; in case it fixes
the syzcaller failure. If that passes, maybe the conn->mutex locking
overlooks something that I don't see right now,

#syz test

> > > This can lead to use-after-free and list corruption bugs, as reported
> > > by syzbot.
> > > 
> > > Fix this by changing l2cap_register_user() and l2cap_unregister_user()
> > > to use conn->lock instead of hci_dev_lock(), ensuring consistent locking
> > > for the l2cap_conn structure.
> > > Reported-by: syzbot+14b6d57fb728e27ce23c@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=14b6d57fb728e27ce23c
> > > Fixes: ab4eedb790ca ("Bluetooth: L2CAP: Fix corrupted list in hci_chan_del")
> > > Signed-off-by: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
> > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > ---
> > >  net/bluetooth/l2cap_core.c | 20 ++++++++------------
> > >  1 file changed, 8 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > > index 14131e427efd..6606d7f12534 100644
> > > --- a/net/bluetooth/l2cap_core.c
> > > +++ b/net/bluetooth/l2cap_core.c
> > > @@ -1678,17 +1678,15 @@ static void l2cap_info_timeout(struct work_struct *work)
> > >  
> > >  int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user)
> > >  {
> > > -	struct hci_dev *hdev = conn->hcon->hdev;
> > >  	int ret;
> > >  
> > >  	/* We need to check whether l2cap_conn is registered. If it is not, we
> > > -	 * must not register the l2cap_user. l2cap_conn_del() is unregisters
> > > -	 * l2cap_conn objects, but doesn't provide its own locking. Instead, it
> > > -	 * relies on the parent hci_conn object to be locked. This itself relies
> > > -	 * on the hci_dev object to be locked. So we must lock the hci device
> > > -	 * here, too. */
> > > +	 * must not register the l2cap_user. l2cap_conn_del() unregisters
> > > +	 * l2cap_conn objects under conn->lock, and we use the same lock here
> > > +	 * to protect access to conn->users and conn->hchan.
> > > +	 */
> > >  
> > > -	hci_dev_lock(hdev);
> > > +	mutex_lock(&conn->lock);
> > >  
> > >  	if (!list_empty(&user->list)) {
> > >  		ret = -EINVAL;
> > > @@ -1709,16 +1707,14 @@ int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user)
> > >  	ret = 0;
> > >  
> > >  out_unlock:
> > > -	hci_dev_unlock(hdev);
> > > +	mutex_unlock(&conn->lock);
> > >  	return ret;
> > >  }
> > >  EXPORT_SYMBOL(l2cap_register_user);
> > >  
> > >  void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user)
> > >  {
> > > -	struct hci_dev *hdev = conn->hcon->hdev;
> > > -
> > > -	hci_dev_lock(hdev);
> > > +	mutex_lock(&conn->lock);
> > >  
> > >  	if (list_empty(&user->list))
> > >  		goto out_unlock;
> > > @@ -1727,7 +1723,7 @@ void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user)
> > >  	user->remove(conn, user);
> > >  
> > >  out_unlock:
> > > -	hci_dev_unlock(hdev);
> > > +	mutex_unlock(&conn->lock);
> > >  }
> > >  EXPORT_SYMBOL(l2cap_unregister_user);
> > >