public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] bluetooth 2026-03-12
@ 2026-03-12 20:06 Luiz Augusto von Dentz
  2026-03-14 16:47 ` Jakub Kicinski
  2026-03-14 17:44 ` Jakub Kicinski
  0 siblings, 2 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2026-03-12 20:06 UTC (permalink / raw)
  To: davem, kuba; +Cc: linux-bluetooth, netdev

The following changes since commit c38b8f5f791ecce13ab77e2257f8fd2444ba80f6:

  net: prevent NULL deref in ip[6]tunnel_xmit() (2026-03-12 16:03:41 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git tags/for-net-2026-03-12

for you to fetch changes up to 99b2c531e0e797119ae1b9195a8764ee98b00e65:

  Bluetooth: qca: fix ROM version reading on WCN3998 chips (2026-03-12 15:29:29 -0400)

----------------------------------------------------------------
bluetooth pull request for net:

 - hci_sync: Fix hci_le_create_conn_sync
 - MGMT: Fix list corruption and UAF in command complete handlers
 - L2CAP: Disconnect if received packet's SDU exceeds IMTU
 - L2CAP: Disconnect if sum of payload sizes exceed SDU
 - L2CAP: Fix accepting multiple L2CAP_ECRED_CONN_REQ
 - L2CAP: Fix type confusion in l2cap_ecred_reconf_rsp()
 - L2CAP: Validate L2CAP_INFO_RSP payload length before access
 - L2CAP: Fix use-after-free in l2cap_unregister_user
 - ISO: Fix defer tests being unstable
 - HIDP: Fix possible UAF
 - SMP: make SM/PER/KDU/BI-04-C happy
 - qca: fix ROM version reading on WCN3998 chips

----------------------------------------------------------------
Christian Eggers (3):
      Bluetooth: LE L2CAP: Disconnect if received packet's SDU exceeds IMTU
      Bluetooth: LE L2CAP: Disconnect if sum of payload sizes exceed SDU
      Bluetooth: SMP: make SM/PER/KDU/BI-04-C happy

Dmitry Baryshkov (1):
      Bluetooth: qca: fix ROM version reading on WCN3998 chips

Luiz Augusto von Dentz (3):
      Bluetooth: ISO: Fix defer tests being unstable
      Bluetooth: HIDP: Fix possible UAF
      Bluetooth: L2CAP: Fix accepting multiple L2CAP_ECRED_CONN_REQ

Lukas Johannes Möller (2):
      Bluetooth: L2CAP: Fix type confusion in l2cap_ecred_reconf_rsp()
      Bluetooth: L2CAP: Validate L2CAP_INFO_RSP payload length before access

Michael Grzeschik (1):
      Bluetooth: hci_sync: Fix hci_le_create_conn_sync

Shaurya Rane (1):
      Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user

Wang Tao (1):
      Bluetooth: MGMT: Fix list corruption and UAF in command complete handlers

 drivers/bluetooth/btqca.c  |  2 ++
 net/bluetooth/hci_conn.c   |  4 ++--
 net/bluetooth/hci_sync.c   |  2 +-
 net/bluetooth/hidp/core.c  | 16 +++++++++++++--
 net/bluetooth/l2cap_core.c | 51 ++++++++++++++++++++++++++++------------------
 net/bluetooth/mgmt.c       |  7 ++-----
 net/bluetooth/smp.c        |  2 +-
 7 files changed, 53 insertions(+), 31 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-12 20:06 [GIT PULL] bluetooth 2026-03-12 Luiz Augusto von Dentz
@ 2026-03-14 16:47 ` Jakub Kicinski
  2026-03-16 13:58   ` Luiz Augusto von Dentz
  2026-03-14 17:44 ` Jakub Kicinski
  1 sibling, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-03-14 16:47 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: davem, linux-bluetooth, netdev

On Thu, 12 Mar 2026 16:06:54 -0400 Luiz Augusto von Dentz wrote:
>  - hci_sync: Fix hci_le_create_conn_sync
>  - MGMT: Fix list corruption and UAF in command complete handlers
>  - L2CAP: Disconnect if received packet's SDU exceeds IMTU
>  - L2CAP: Disconnect if sum of payload sizes exceed SDU
>  - L2CAP: Fix accepting multiple L2CAP_ECRED_CONN_REQ
>  - L2CAP: Fix type confusion in l2cap_ecred_reconf_rsp()
>  - L2CAP: Validate L2CAP_INFO_RSP payload length before access
>  - L2CAP: Fix use-after-free in l2cap_unregister_user
>  - ISO: Fix defer tests being unstable
>  - HIDP: Fix possible UAF
>  - SMP: make SM/PER/KDU/BI-04-C happy
>  - qca: fix ROM version reading on WCN3998 chips

FWIW this is what our AI had to say about the series, mostly nit picks
but not all (LMK if you want me to hook the AI reviewer up to your
patchwork):

================================================================================
PATCH 1/12
================================================================================
commit e1d9a66889867c232657a9b6f25d451d7c3ab96f
Author: Christian Eggers <ceggers@arri.de>

Bluetooth: LE L2CAP: Disconnect if received packet's SDU exceeds IMTU

This commit adds proper validation and disconnect handling when received
L2CAP SDU length exceeds the receiver's MTU, fixing compliance test
L2CAP/LE/CFC/BV-26-C.

> Fixes: aac23bf63659 ("Bluetooth: Implement LE L2CAP reassembly")

The Fixes: tag points to commit aac23bf63659 ("Bluetooth: Implement LE
L2CAP reassembly"), but the function being fixed, l2cap_ecred_data_rcv(),
was created in commit 15f02b910562 ("Bluetooth: L2CAP: Add initial code
for Enhanced Credit Based Mode"). While aac23bf63659 introduced a similar
bug in the older l2cap_le_data_rcv() function, shouldn't the Fixes: tag
point to the commit that introduced the specific function being fixed?

Suggested: Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for
Enhanced Credit Based Mode")

> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ad98db9632fd..3056dcd5fa2f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c

[ ... ]

> @@ -6689,7 +6691,9 @@ static int l2cap_ecred_data_rcv(struct l2cap_chan *chan, struct sk_buff *skb)
>  		       sdu_len, skb->len, chan->imtu);
>
>  		if (sdu_len > chan->imtu) {
> -			BT_ERR("Too big LE L2CAP SDU length received");
> +			BT_ERR("Too big LE L2CAP SDU length: len %u > %u",
> +			       skb->len, sdu_len);
> +			l2cap_send_disconn_req(chan, ECONNRESET);
>  			err = -EMSGSIZE;
>  			goto failed;
>  		}

The error message prints skb->len and sdu_len, but the condition checks
sdu_len > chan->imtu. This is inconsistent with the first hunk of this
patch, which correctly prints the two values being compared:

net/bluetooth/l2cap_core.c:l2cap_ecred_data_rcv() {
    if (skb->len > chan->imtu) {
        BT_ERR("Too big LE L2CAP PDU: len %u > %u", skb->len,
               chan->imtu);
        ...
    }
}

Shouldn't the second error message follow the same pattern and print
sdu_len and chan->imtu instead?



================================================================================
PATCH 2/12
================================================================================
commit b6a2bf43aa37670432843bc73ae2a6288ba4d6f8
Author: Christian Eggers <ceggers@arri.de>

Bluetooth: LE L2CAP: Disconnect if sum of payload sizes exceed SDU

This commit adds a disconnect call when the sum of payload sizes for
K-frames exceeds the specified SDU length, addressing test case
L2CAP/LE/CFC/BV-27-C as specified in Core 6.0, Vol 3, Part A, 3.4.3.

> Fixes: aac23bf63659 ("Bluetooth: Implement LE L2CAP reassembly")

Does the Fixes: tag point to the correct commit? The current fix modifies
l2cap_ecred_data_rcv(), which was introduced in commit 15f02b910562
("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode"),
not in aac23bf63659.

Should the Fixes: tag be: Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add
initial code for Enhanced Credit Based Mode")?



================================================================================
PATCH 3/12 - No review comments
================================================================================

================================================================================
PATCH 4/12 - No review comments
================================================================================

================================================================================
PATCH 5/12 - No review comments
================================================================================

================================================================================
PATCH 6/12
================================================================================
commit 17f89341cb4281d1da0e2fb0de5406ab7c4e25ef
Author: Wang Tao <wangtao554@huawei.com>

Bluetooth: MGMT: Fix list corruption and UAF in command complete handlers

This commit addresses two issues from the 302a1f674c00 rework that
changed mgmt_pending_valid() semantics: fixing double list_del() in
mgmt_add_adv_patterns_monitor_complete() and removing incorrect
mgmt_pending_foreach() usage in set_mesh_complete().

> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index a7238fd3b03b..d52238ce6a9a 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c

[ ... ]

> @@ -5377,7 +5374,7 @@ static void mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev,
>
>  	mgmt_cmd_complete(cmd->sk, cmd->hdev->id, cmd->opcode,
>  			  mgmt_status(status), &rp, sizeof(rp));
> -	mgmt_pending_remove(cmd);
> +	mgmt_pending_free(cmd);
>
>  	hci_dev_unlock(hdev);
>  	bt_dev_dbg(hdev, "add monitor %d complete, status %d",

Is this change actually correct? Looking at the function entry condition:

    if (status == -ECANCELED && !mgmt_pending_valid(hdev, cmd))
        return;

Due to short-circuit evaluation of '&&', mgmt_pending_valid() is only
called when status == -ECANCELED. On the normal completion path (status !=
-ECANCELED), mgmt_pending_valid() is never executed, so cmd remains linked
in hdev->mgmt_pending.

Calling mgmt_pending_free(cmd) here would kfree(cmd) without unlinking it
from the list first, leaving a dangling pointer. Any subsequent list
traversal (e.g., mgmt_pending_foreach during __mgmt_power_off, or another
mgmt_pending_valid call) would dereference freed memory.

Compare this to other MGMT completion handlers which use '||' instead:

    if (err == -ECANCELED || !mgmt_pending_valid(hdev, cmd))
        return;

With '||', mgmt_pending_valid() always executes on the normal path and
unlinks cmd from the list, making mgmt_pending_free(cmd) safe. Should this
function's early return condition be changed to use '||' to match the other
handlers?



================================================================================
PATCH 7/12
================================================================================
commit dbf666e4fc9bdd975a61bf682b3f75cb0145eedd
Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Bluetooth: HIDP: Fix possible UAF

This fixes the use-after-free caused by not dropping the l2cap_conn
reference when the user->remove callback is called. The patch drops the
l2cap_conn reference in hidp_session_remove() and adds NULL checks in
both session_free() and hidp_session_thread().

> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 6fe815241b01..7bcf8c5ceaee 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c

[ ... ]

> @@ -1164,6 +1165,15 @@ static void hidp_session_remove(struct l2cap_conn *conn,
>
>  	down_write(&hidp_session_sem);
>
> +	/* Drop L2CAP reference immediately to indicate that
> +	 * l2cap_unregister_user() shall not be called as it is already
> +	 * considered removed.
> +	 */
> +	if (session->conn) {
> +		l2cap_conn_put(session->conn);
> +		session->conn = NULL;
> +	}
> +
>  	hidp_session_terminate(session);
>
>  	cancel_work_sync(&session->dev_init);

Does setting session->conn = NULL in hidp_session_remove() create a
race with hidp_connection_del()?

Before this patch, session->conn was always a valid pointer (initialized
in hidp_session_new() and only released in session_free()). After this
patch, hidp_session_remove() sets session->conn = NULL.

Looking at hidp_connection_del():

int hidp_connection_del(struct hidp_conndel_req *req)
{
	...
	session = hidp_session_find(&req->bdaddr);
	if (!session)
		return -ENOENT;

	if (req->flags & BIT(HIDP_VIRTUAL_CABLE_UNPLUG))
		hidp_send_ctrl_message(session, ...);
	else
		l2cap_unregister_user(session->conn, &session->user);

	hidp_session_put(session);
	return 0;
}

After hidp_session_find() releases hidp_session_sem, there's no
protection preventing hidp_session_remove() from running concurrently and
setting session->conn = NULL.

If hidp_session_remove() runs between the hidp_session_find() return and
the l2cap_unregister_user() call, then l2cap_unregister_user() receives a
NULL conn pointer and immediately dereferences it in its first line:

	struct hci_dev *hdev = conn->hcon->hdev;

causing a NULL pointer dereference.

> @@ -1301,7 +1311,9 @@ static int hidp_session_thread(void *arg)
>  	 * Instead, this call has the same semantics as if user-space tried to
>  	 * delete the session.
>  	 */
> -	l2cap_unregister_user(session->conn, &session->user);
> +	if (session->conn)
> +		l2cap_unregister_user(session->conn, &session->user);
> +
>  	hidp_session_put(session);
>
>  	module_put_and_kthread_exit(0);




================================================================================
PATCH 8/12
================================================================================
commit 752a6c9596dd25efd6978a73ff21f3b592668f4a
Author: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>

Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user

This patch addresses a use-after-free bug reported by syzbot where
l2cap_unregister_user() dereferences conn->hcon->hdev after the hci_dev
structure has been freed. The fix switches from hci_dev_lock to conn->lock
in both l2cap_register_user() and l2cap_unregister_user().

Closes: https://syzkaller.appspot.com/bug?extid=14b6d57fb728e27ce23c

> Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user
>
> After commit ab4eedb790ca ("Bluetooth: L2CAP: Fix corrupted list in
> hci_chan_del"), l2cap_conn_del() uses conn->lock to protect access to
> conn->users. However, l2cap_register_user() and l2cap_unregister_user()
> don't use conn->lock, creating a race condition where these functions can
> access conn->users and conn->hchan concurrently with l2cap_conn_del().
>
> This can lead to use-after-free and list corruption bugs, as reported
> by syzbot.

Does the commit message accurately describe the syzbot-reported bug?

The syzbot report (syzbot+14b6d57fb728e27ce23c) shows a use-after-free in
l2cap_unregister_user at the mutex_lock_nested call. The actual crash
happens because the hci_dev structure (accessed via conn->hcon->hdev)
was already freed when l2cap_unregister_user tried to lock it with
hci_dev_lock(hdev):

Crash site:
  l2cap_unregister_user -> hci_dev_lock(hdev)
  where hdev = conn->hcon->hdev

Freed by:
  hci_release_dev -> kfree()
  via vhci_release -> hci_free_dev -> kobject_put -> hci_release_dev

The freed object is an 8192-byte kmalloc slab (hci_dev), and the crash
task is khidpd_* (hidp_session_thread).

The commit message states "This can lead to use-after-free and list
corruption bugs, as reported by syzbot." While the theoretical race on
conn->users IS possible, the actual syzbot-reported crash is about
accessing freed hdev memory when trying to acquire hci_dev_lock, not
about conn->users list corruption or conn->hchan use-after-free.

>
> 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>
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 0f400051f093..780136e18aae 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.
> +	 */

Does the new comment fully explain the reason for the change?

The comment states that l2cap_conn_del() unregisters l2cap_conn objects
under conn->lock, and we use the same lock to protect conn->users and
conn->hchan. This is factually correct.

However, the comment omits the actual reason for changing the locking:
the old code dereferenced conn->hcon->hdev to acquire hci_dev_lock,
but hci_dev can be freed before this point, causing the use-after-free
crash that syzbot reported.

The comment describes a secondary benefit (consistent locking for
conn->users and conn->hchan) rather than the primary bug being fixed
(accessing a freed hci_dev structure through conn->hcon->hdev).

>
> -	hci_dev_lock(hdev);
> +	mutex_lock(&conn->lock);
>
>  	if (!list_empty(&user->list)) {
>  		ret = -EINVAL;

[ ... ]



================================================================================
PATCH 9/12 - No review comments
================================================================================

================================================================================
PATCH 10/12 - No review comments
================================================================================

================================================================================
PATCH 11/12 - No review comments
================================================================================

================================================================================
PATCH 12/12 - No review comments
================================================================================

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-12 20:06 [GIT PULL] bluetooth 2026-03-12 Luiz Augusto von Dentz
  2026-03-14 16:47 ` Jakub Kicinski
@ 2026-03-14 17:44 ` Jakub Kicinski
  2026-03-15 15:04   ` Konstantin Ryabitsev
  1 sibling, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-03-14 17:44 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, helpdesk; +Cc: davem, linux-bluetooth, netdev

On Thu, 12 Mar 2026 16:06:54 -0400 Luiz Augusto von Dentz wrote:
> The following changes since commit c38b8f5f791ecce13ab77e2257f8fd2444ba80f6:
> 
>   net: prevent NULL deref in ip[6]tunnel_xmit() (2026-03-12 16:03:41 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git tags/for-net-2026-03-12
> 
> for you to fetch changes up to 99b2c531e0e797119ae1b9195a8764ee98b00e65:
> 
>   Bluetooth: qca: fix ROM version reading on WCN3998 chips (2026-03-12 15:29:29 -0400)

To be clear I have pulled already, if any of the AI reports looks
urgent please send a follow up.

Hello helpdesk! The patchwork reply bot has not been replying to pull
requests for us in the last few weeks. Is this an intentional change?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-14 17:44 ` Jakub Kicinski
@ 2026-03-15 15:04   ` Konstantin Ryabitsev
  2026-03-15 20:20     ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-15 15:04 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Luiz Augusto von Dentz, helpdesk, davem, linux-bluetooth, netdev

On Sat, Mar 14, 2026 at 10:44:36AM -0700, Jakub Kicinski wrote:
> To be clear I have pulled already, if any of the AI reports looks
> urgent please send a follow up.
> 
> Hello helpdesk! The patchwork reply bot has not been replying to pull
> requests for us in the last few weeks. Is this an intentional change?

It was an intentional change to make it work more reliably, but we clearly
failed because it stopped matching altogether. *sigh*

I think I found where the match was failing and added tests to check for it.
It won't work retroactively, but it should find future pull requests
(hopefully!)

Regards,
-- 
KR

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-15 15:04   ` Konstantin Ryabitsev
@ 2026-03-15 20:20     ` Jakub Kicinski
  2026-03-19  2:48       ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-03-15 20:20 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Luiz Augusto von Dentz, helpdesk, davem, linux-bluetooth, netdev

On Sun, 15 Mar 2026 11:04:47 -0400 Konstantin Ryabitsev wrote:
> On Sat, Mar 14, 2026 at 10:44:36AM -0700, Jakub Kicinski wrote:
> > To be clear I have pulled already, if any of the AI reports looks
> > urgent please send a follow up.
> > 
> > Hello helpdesk! The patchwork reply bot has not been replying to pull
> > requests for us in the last few weeks. Is this an intentional change?  
> 
> It was an intentional change to make it work more reliably, but we clearly
> failed because it stopped matching altogether. *sigh*
> 
> I think I found where the match was failing and added tests to check for it.
> It won't work retroactively, but it should find future pull requests
> (hopefully!)

Thank you! I'll report back in a couple of days.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-14 16:47 ` Jakub Kicinski
@ 2026-03-16 13:58   ` Luiz Augusto von Dentz
  2026-03-16 23:26     ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2026-03-16 13:58 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, linux-bluetooth, netdev

Hi Jakub,

On Sat, Mar 14, 2026 at 12:47 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 12 Mar 2026 16:06:54 -0400 Luiz Augusto von Dentz wrote:
> >  - hci_sync: Fix hci_le_create_conn_sync
> >  - MGMT: Fix list corruption and UAF in command complete handlers
> >  - L2CAP: Disconnect if received packet's SDU exceeds IMTU
> >  - L2CAP: Disconnect if sum of payload sizes exceed SDU
> >  - L2CAP: Fix accepting multiple L2CAP_ECRED_CONN_REQ
> >  - L2CAP: Fix type confusion in l2cap_ecred_reconf_rsp()
> >  - L2CAP: Validate L2CAP_INFO_RSP payload length before access
> >  - L2CAP: Fix use-after-free in l2cap_unregister_user
> >  - ISO: Fix defer tests being unstable
> >  - HIDP: Fix possible UAF
> >  - SMP: make SM/PER/KDU/BI-04-C happy
> >  - qca: fix ROM version reading on WCN3998 chips
>
> FWIW this is what our AI had to say about the series, mostly nit picks
> but not all (LMK if you want me to hook the AI reviewer up to your
> patchwork):

Sure, I like to receive reviews of that, note though that
linux-bluetooth and patchwork are for both kernel and userspace, so we
may need so quirks for detecting if a set is for kernel or userspace.

>
> ================================================================================
> PATCH 1/12
> ================================================================================
> commit e1d9a66889867c232657a9b6f25d451d7c3ab96f
> Author: Christian Eggers <ceggers@arri.de>
>
> Bluetooth: LE L2CAP: Disconnect if received packet's SDU exceeds IMTU
>
> This commit adds proper validation and disconnect handling when received
> L2CAP SDU length exceeds the receiver's MTU, fixing compliance test
> L2CAP/LE/CFC/BV-26-C.
>
> > Fixes: aac23bf63659 ("Bluetooth: Implement LE L2CAP reassembly")
>
> The Fixes: tag points to commit aac23bf63659 ("Bluetooth: Implement LE
> L2CAP reassembly"), but the function being fixed, l2cap_ecred_data_rcv(),
> was created in commit 15f02b910562 ("Bluetooth: L2CAP: Add initial code
> for Enhanced Credit Based Mode"). While aac23bf63659 introduced a similar
> bug in the older l2cap_le_data_rcv() function, shouldn't the Fixes: tag
> point to the commit that introduced the specific function being fixed?
>
> Suggested: Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for
> Enhanced Credit Based Mode")
>
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index ad98db9632fd..3056dcd5fa2f 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
>
> [ ... ]
>
> > @@ -6689,7 +6691,9 @@ static int l2cap_ecred_data_rcv(struct l2cap_chan *chan, struct sk_buff *skb)
> >                      sdu_len, skb->len, chan->imtu);
> >
> >               if (sdu_len > chan->imtu) {
> > -                     BT_ERR("Too big LE L2CAP SDU length received");
> > +                     BT_ERR("Too big LE L2CAP SDU length: len %u > %u",
> > +                            skb->len, sdu_len);
> > +                     l2cap_send_disconn_req(chan, ECONNRESET);
> >                       err = -EMSGSIZE;
> >                       goto failed;
> >               }
>
> The error message prints skb->len and sdu_len, but the condition checks
> sdu_len > chan->imtu. This is inconsistent with the first hunk of this
> patch, which correctly prints the two values being compared:
>
> net/bluetooth/l2cap_core.c:l2cap_ecred_data_rcv() {
>     if (skb->len > chan->imtu) {
>         BT_ERR("Too big LE L2CAP PDU: len %u > %u", skb->len,
>                chan->imtu);
>         ...
>     }
> }
>
> Shouldn't the second error message follow the same pattern and print
> sdu_len and chan->imtu instead?

Not sure I follow this one, both changes actually do print in similar style:

@@ -6689,7 +6691,9 @@ static int l2cap_ecred_data_rcv(struct
l2cap_chan *chan, struct sk_buff *skb)
                       sdu_len, skb->len, chan->imtu);

                if (sdu_len > chan->imtu) {
-                       BT_ERR("Too big LE L2CAP SDU length received");
+                       BT_ERR("Too big LE L2CAP SDU length: len %u > %u",
+                              skb->len, sdu_len);
+                       l2cap_send_disconn_req(chan, ECONNRESET);
                        err = -EMSGSIZE;
                        goto failed;
                }

Oh, wait a minute, it is printing skb->len rather than sdu_len and the
second portion is also wrong, it should have been imtu.

>
>
> ================================================================================
> PATCH 2/12
> ================================================================================
> commit b6a2bf43aa37670432843bc73ae2a6288ba4d6f8
> Author: Christian Eggers <ceggers@arri.de>
>
> Bluetooth: LE L2CAP: Disconnect if sum of payload sizes exceed SDU
>
> This commit adds a disconnect call when the sum of payload sizes for
> K-frames exceeds the specified SDU length, addressing test case
> L2CAP/LE/CFC/BV-27-C as specified in Core 6.0, Vol 3, Part A, 3.4.3.
>
> > Fixes: aac23bf63659 ("Bluetooth: Implement LE L2CAP reassembly")
>
> Does the Fixes: tag point to the correct commit? The current fix modifies
> l2cap_ecred_data_rcv(), which was introduced in commit 15f02b910562
> ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode"),
> not in aac23bf63659.
>
> Should the Fixes: tag be: Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add
> initial code for Enhanced Credit Based Mode")?

False positive, this only really apply after aac23bf63659.

>
>
> ================================================================================
> PATCH 3/12 - No review comments
> ================================================================================
>
> ================================================================================
> PATCH 4/12 - No review comments
> ================================================================================
>
> ================================================================================
> PATCH 5/12 - No review comments
> ================================================================================
>
> ================================================================================
> PATCH 6/12
> ================================================================================
> commit 17f89341cb4281d1da0e2fb0de5406ab7c4e25ef
> Author: Wang Tao <wangtao554@huawei.com>
>
> Bluetooth: MGMT: Fix list corruption and UAF in command complete handlers
>
> This commit addresses two issues from the 302a1f674c00 rework that
> changed mgmt_pending_valid() semantics: fixing double list_del() in
> mgmt_add_adv_patterns_monitor_complete() and removing incorrect
> mgmt_pending_foreach() usage in set_mesh_complete().
>
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index a7238fd3b03b..d52238ce6a9a 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
>
> [ ... ]
>
> > @@ -5377,7 +5374,7 @@ static void mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev,
> >
> >       mgmt_cmd_complete(cmd->sk, cmd->hdev->id, cmd->opcode,
> >                         mgmt_status(status), &rp, sizeof(rp));
> > -     mgmt_pending_remove(cmd);
> > +     mgmt_pending_free(cmd);
> >
> >       hci_dev_unlock(hdev);
> >       bt_dev_dbg(hdev, "add monitor %d complete, status %d",
>
> Is this change actually correct? Looking at the function entry condition:
>
>     if (status == -ECANCELED && !mgmt_pending_valid(hdev, cmd))
>         return;
>
> Due to short-circuit evaluation of '&&', mgmt_pending_valid() is only
> called when status == -ECANCELED. On the normal completion path (status !=
> -ECANCELED), mgmt_pending_valid() is never executed, so cmd remains linked
> in hdev->mgmt_pending.
>
> Calling mgmt_pending_free(cmd) here would kfree(cmd) without unlinking it
> from the list first, leaving a dangling pointer. Any subsequent list
> traversal (e.g., mgmt_pending_foreach during __mgmt_power_off, or another
> mgmt_pending_valid call) would dereference freed memory.
>
> Compare this to other MGMT completion handlers which use '||' instead:
>
>     if (err == -ECANCELED || !mgmt_pending_valid(hdev, cmd))
>         return;
>
> With '||', mgmt_pending_valid() always executes on the normal path and
> unlinks cmd from the list, making mgmt_pending_free(cmd) safe. Should this
> function's early return condition be changed to use '||' to match the other
> handlers?

That is an issue indeed, ack.

>
>
> ================================================================================
> PATCH 7/12
> ================================================================================
> commit dbf666e4fc9bdd975a61bf682b3f75cb0145eedd
> Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> Bluetooth: HIDP: Fix possible UAF
>
> This fixes the use-after-free caused by not dropping the l2cap_conn
> reference when the user->remove callback is called. The patch drops the
> l2cap_conn reference in hidp_session_remove() and adds NULL checks in
> both session_free() and hidp_session_thread().
>
> > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> > index 6fe815241b01..7bcf8c5ceaee 100644
> > --- a/net/bluetooth/hidp/core.c
> > +++ b/net/bluetooth/hidp/core.c
>
> [ ... ]
>
> > @@ -1164,6 +1165,15 @@ static void hidp_session_remove(struct l2cap_conn *conn,
> >
> >       down_write(&hidp_session_sem);
> >
> > +     /* Drop L2CAP reference immediately to indicate that
> > +      * l2cap_unregister_user() shall not be called as it is already
> > +      * considered removed.
> > +      */
> > +     if (session->conn) {
> > +             l2cap_conn_put(session->conn);
> > +             session->conn = NULL;
> > +     }
> > +
> >       hidp_session_terminate(session);
> >
> >       cancel_work_sync(&session->dev_init);
>
> Does setting session->conn = NULL in hidp_session_remove() create a
> race with hidp_connection_del()?
>
> Before this patch, session->conn was always a valid pointer (initialized
> in hidp_session_new() and only released in session_free()). After this
> patch, hidp_session_remove() sets session->conn = NULL.
>
> Looking at hidp_connection_del():
>
> int hidp_connection_del(struct hidp_conndel_req *req)
> {
>         ...
>         session = hidp_session_find(&req->bdaddr);
>         if (!session)
>                 return -ENOENT;
>
>         if (req->flags & BIT(HIDP_VIRTUAL_CABLE_UNPLUG))
>                 hidp_send_ctrl_message(session, ...);
>         else
>                 l2cap_unregister_user(session->conn, &session->user);
>
>         hidp_session_put(session);
>         return 0;
> }
>
> After hidp_session_find() releases hidp_session_sem, there's no
> protection preventing hidp_session_remove() from running concurrently and
> setting session->conn = NULL.
>
> If hidp_session_remove() runs between the hidp_session_find() return and
> the l2cap_unregister_user() call, then l2cap_unregister_user() receives a
> NULL conn pointer and immediately dereferences it in its first line:
>
>         struct hci_dev *hdev = conn->hcon->hdev;
>
> causing a NULL pointer dereference.
>
> > @@ -1301,7 +1311,9 @@ static int hidp_session_thread(void *arg)
> >        * Instead, this call has the same semantics as if user-space tried to
> >        * delete the session.
> >        */
> > -     l2cap_unregister_user(session->conn, &session->user);
> > +     if (session->conn)
> > +             l2cap_unregister_user(session->conn, &session->user);
> > +
> >       hidp_session_put(session);
> >
> >       module_put_and_kthread_exit(0);

Ok, this shouldn't have been merged, the follow up patch fixes the
same issue, although perhaps having them both sort of garantee
hidp_session_remove and hidp_connection_del cannot run concurrently.

>
>
>
> ================================================================================
> PATCH 8/12
> ================================================================================
> commit 752a6c9596dd25efd6978a73ff21f3b592668f4a
> Author: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
>
> Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user
>
> This patch addresses a use-after-free bug reported by syzbot where
> l2cap_unregister_user() dereferences conn->hcon->hdev after the hci_dev
> structure has been freed. The fix switches from hci_dev_lock to conn->lock
> in both l2cap_register_user() and l2cap_unregister_user().
>
> Closes: https://syzkaller.appspot.com/bug?extid=14b6d57fb728e27ce23c
>
> > Bluetooth: L2CAP: Fix use-after-free in l2cap_unregister_user
> >
> > After commit ab4eedb790ca ("Bluetooth: L2CAP: Fix corrupted list in
> > hci_chan_del"), l2cap_conn_del() uses conn->lock to protect access to
> > conn->users. However, l2cap_register_user() and l2cap_unregister_user()
> > don't use conn->lock, creating a race condition where these functions can
> > access conn->users and conn->hchan concurrently with l2cap_conn_del().
> >
> > This can lead to use-after-free and list corruption bugs, as reported
> > by syzbot.
>
> Does the commit message accurately describe the syzbot-reported bug?
>
> The syzbot report (syzbot+14b6d57fb728e27ce23c) shows a use-after-free in
> l2cap_unregister_user at the mutex_lock_nested call. The actual crash
> happens because the hci_dev structure (accessed via conn->hcon->hdev)
> was already freed when l2cap_unregister_user tried to lock it with
> hci_dev_lock(hdev):
>
> Crash site:
>   l2cap_unregister_user -> hci_dev_lock(hdev)
>   where hdev = conn->hcon->hdev
>
> Freed by:
>   hci_release_dev -> kfree()
>   via vhci_release -> hci_free_dev -> kobject_put -> hci_release_dev
>
> The freed object is an 8192-byte kmalloc slab (hci_dev), and the crash
> task is khidpd_* (hidp_session_thread).
>
> The commit message states "This can lead to use-after-free and list
> corruption bugs, as reported by syzbot." While the theoretical race on
> conn->users IS possible, the actual syzbot-reported crash is about
> accessing freed hdev memory when trying to acquire hci_dev_lock, not
> about conn->users list corruption or conn->hchan use-after-free.
>
> >
> > 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>
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 0f400051f093..780136e18aae 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.
> > +      */
>
> Does the new comment fully explain the reason for the change?
>
> The comment states that l2cap_conn_del() unregisters l2cap_conn objects
> under conn->lock, and we use the same lock to protect conn->users and
> conn->hchan. This is factually correct.
>
> However, the comment omits the actual reason for changing the locking:
> the old code dereferenced conn->hcon->hdev to acquire hci_dev_lock,
> but hci_dev can be freed before this point, causing the use-after-free
> crash that syzbot reported.
>
> The comment describes a secondary benefit (consistent locking for
> conn->users and conn->hchan) rather than the primary bug being fixed
> (accessing a freed hci_dev structure through conn->hcon->hdev).

Might need to update the docunentation, but it is not incorrect.

> >
> > -     hci_dev_lock(hdev);
> > +     mutex_lock(&conn->lock);
> >
> >       if (!list_empty(&user->list)) {
> >               ret = -EINVAL;
>
> [ ... ]
>
>
>
> ================================================================================
> PATCH 9/12 - No review comments
> ================================================================================
>
> ================================================================================
> PATCH 10/12 - No review comments
> ================================================================================
>
> ================================================================================
> PATCH 11/12 - No review comments
> ================================================================================
>
> ================================================================================
> PATCH 12/12 - No review comments
> ================================================================================



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-16 13:58   ` Luiz Augusto von Dentz
@ 2026-03-16 23:26     ` Jakub Kicinski
  2026-03-17 20:45       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-03-16 23:26 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: davem, linux-bluetooth, netdev

On Mon, 16 Mar 2026 09:58:56 -0400 Luiz Augusto von Dentz wrote:
> > FWIW this is what our AI had to say about the series, mostly nit picks
> > but not all (LMK if you want me to hook the AI reviewer up to your
> > patchwork):  
> 
> Sure, I like to receive reviews of that, note though that
> linux-bluetooth and patchwork are for both kernel and userspace, so we
> may need so quirks for detecting if a set is for kernel or userspace.

I think similar thing happens for iouring.
The user space patches just fail to apply and the LLM doesn't run.
If that's alright with you please ask helpdesk@ to give the "nipa"
user permissions to post checks to your patchwork instance and LMK
when ready, I'll hook it up.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-16 23:26     ` Jakub Kicinski
@ 2026-03-17 20:45       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2026-03-17 20:45 UTC (permalink / raw)
  To: Jakub Kicinski, helpdesk; +Cc: davem, linux-bluetooth, netdev

Hi Helpdesk,

On Mon, Mar 16, 2026 at 7:26 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 16 Mar 2026 09:58:56 -0400 Luiz Augusto von Dentz wrote:
> > > FWIW this is what our AI had to say about the series, mostly nit picks
> > > but not all (LMK if you want me to hook the AI reviewer up to your
> > > patchwork):
> >
> > Sure, I like to receive reviews of that, note though that
> > linux-bluetooth and patchwork are for both kernel and userspace, so we
> > may need so quirks for detecting if a set is for kernel or userspace.
>
> I think similar thing happens for iouring.
> The user space patches just fail to apply and the LLM doesn't run.
> If that's alright with you please ask helpdesk@ to give the "nipa"
> user permissions to post checks to your patchwork instance and LMK
> when ready, I'll hook it up.

Can you guys help me with the above?


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-15 20:20     ` Jakub Kicinski
@ 2026-03-19  2:48       ` Jakub Kicinski
  2026-03-19 13:21         ` Konstantin Ryabitsev
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-03-19  2:48 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Luiz Augusto von Dentz, helpdesk, davem, linux-bluetooth, netdev

On Sun, 15 Mar 2026 13:20:13 -0700 Jakub Kicinski wrote:
> On Sun, 15 Mar 2026 11:04:47 -0400 Konstantin Ryabitsev wrote:
> > On Sat, Mar 14, 2026 at 10:44:36AM -0700, Jakub Kicinski wrote:  
> > > To be clear I have pulled already, if any of the AI reports looks
> > > urgent please send a follow up.
> > > 
> > > Hello helpdesk! The patchwork reply bot has not been replying to pull
> > > requests for us in the last few weeks. Is this an intentional change?    
> > 
> > It was an intentional change to make it work more reliably, but we clearly
> > failed because it stopped matching altogether. *sigh*
> > 
> > I think I found where the match was failing and added tests to check for it.
> > It won't work retroactively, but it should find future pull requests
> > (hopefully!)  
> 
> Thank you! I'll report back in a couple of days.

Since I promised - pw-bot replies to pulls seem to be working fine now

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] bluetooth 2026-03-12
  2026-03-19  2:48       ` Jakub Kicinski
@ 2026-03-19 13:21         ` Konstantin Ryabitsev
  0 siblings, 0 replies; 10+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-19 13:21 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Luiz Augusto von Dentz, helpdesk, davem, linux-bluetooth, netdev

On Wed, Mar 18, 2026, at 22:48, Jakub Kicinski wrote:
>> > It won't work retroactively, but it should find future pull requests
>> > (hopefully!)  
>> 
>> Thank you! I'll report back in a couple of days.
>
> Since I promised - pw-bot replies to pulls seem to be working fine now

Great, thanks for following up. 

-K

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-03-19 13:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-12 20:06 [GIT PULL] bluetooth 2026-03-12 Luiz Augusto von Dentz
2026-03-14 16:47 ` Jakub Kicinski
2026-03-16 13:58   ` Luiz Augusto von Dentz
2026-03-16 23:26     ` Jakub Kicinski
2026-03-17 20:45       ` Luiz Augusto von Dentz
2026-03-14 17:44 ` Jakub Kicinski
2026-03-15 15:04   ` Konstantin Ryabitsev
2026-03-15 20:20     ` Jakub Kicinski
2026-03-19  2:48       ` Jakub Kicinski
2026-03-19 13:21         ` Konstantin Ryabitsev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox