* Re: [PATCH BlueZ] avrcp: Abort continuing response on fragmented CT replies
From: patchwork-bot+bluetooth @ 2026-06-10 18:40 UTC (permalink / raw)
To: Simon Mikuda; +Cc: linux-bluetooth
In-Reply-To: <20260609212656.3900190-1-simon.mikuda@streamunlimited.com>
Hello:
This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Tue, 9 Jun 2026 23:26:56 +0200 you wrote:
> Send AbortContinuingResponse when a Get Element Attributes reply
> arrives fragmented, as the CT side does not reassemble fragments.
>
> Fixes PTS test AVRCP/CT/RCR/BV-03-C
> ---
> profiles/audio/avrcp.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
Here is the summary with links:
- [BlueZ] avrcp: Abort continuing response on fragmented CT replies
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=249e2bedfacf
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH BlueZ] avdtp: Return correct error when SEP is inuse
From: patchwork-bot+bluetooth @ 2026-06-10 18:40 UTC (permalink / raw)
To: Simon Mikuda; +Cc: linux-bluetooth
In-Reply-To: <20260609213013.3900808-1-simon.mikuda@streamunlimited.com>
Hello:
This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Tue, 9 Jun 2026 23:30:13 +0200 you wrote:
> This fixes AVDTP/SNK/ACP/SIG/SMG/BI-08-C
> ---
> profiles/audio/avdtp.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Here is the summary with links:
- [BlueZ] avdtp: Return correct error when SEP is inuse
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=39ad431de60b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH BlueZ v2] adapter: Fix failed bonding attempt after LE link disconnection
From: patchwork-bot+bluetooth @ 2026-06-10 18:40 UTC (permalink / raw)
To: Simon Mikuda; +Cc: linux-bluetooth
In-Reply-To: <20260610082054.3915366-1-simon.mikuda@streamunlimited.com>
Hello:
This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Wed, 10 Jun 2026 10:20:54 +0200 you wrote:
> What happens when issue occurs:
> - device is connected to both bearers BR/EDR and LE
> - bonding is requested
> - LE link disconnects
> - pairing keys arrive
>
> BlueZ would finish bonding request with error and mark device as
> temporary. Then it would be disconnected+removed after default
> temporary timeout (30 seconds).
>
> [...]
Here is the summary with links:
- [BlueZ,v2] adapter: Fix failed bonding attempt after LE link disconnection
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5d836f1c697c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: RTL8922A (0bda:d922): BLE HID disconnects with supervision timeout (0x08) during active HFP/SCO call
From: Benjamin Mirko Blume @ 2026-06-10 18:38 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Max Chou
In-Reply-To: <20d6a474-18c9-439b-8b01-a3ef5e8310cd@posteo.de>
[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]
Gentle ping on this one — still reproducing reliably, now on kernel
7.0.11-1-cachyos-bore.
Quoting the core of the original report for context:
> a BLE-only HID device (mouse) is disconnected with link supervision
> timeout whenever an HFP voice call (SCO) is active on the same
> controller. The SCO audio link itself stays up; only the LE HID link
> starves.
>
> HCI Event: Disconnect Complete (0x05) Handle: 16
> Reason: Connection Timeout (0x08)
> MGMT Event: Device Disconnected
> LE Address: DB:5C:42:57:1E:1E (the mouse)
As offered in the original report, I'm attaching the short,
payload-scrubbed btmon reproducer (btmon-rtl8922-repro-scrubbed.snoop).
The raw SCO payload (call audio) has been stripped; the HCI/MGMT command
and event flow — the LE connection events, the eSCO setup, and the
Disconnect Complete with reason 0x08 — is intact, so the starvation of
the LE link during the active SCO call is fully visible in the capture.
To recap the open questions:
- Is this a known coexistence/scheduling limitation of the RTL8922A
firmware, or is the SCO/LE arbitration firmware-tunable?
- Can the host side mitigate it (e.g. via LE connection parameters)?
- Could it have regressed between linux-firmware versions? I'm happy to
bisect linux-firmware, and happy to test any debug patch.
Thanks,
Benjamin
[-- Attachment #2: btmon-rtl8922-repro-scrubbed.snoop --]
[-- Type: application/octet-stream, Size: 232107 bytes --]
^ permalink raw reply
* Re: [PATCH BlueZ v2 1/4] btio: Handle EOPNOTSUPP from accept() to prevent busy loop
From: patchwork-bot+bluetooth @ 2026-06-10 18:30 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <20260609185313.155105-1-luiz.dentz@gmail.com>
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Tue, 9 Jun 2026 14:53:10 -0400 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> When accept() returns EOPNOTSUPP on an L2CAP SEQPACKET server socket
> (e.g. AVCTP browsing channel, PSM 0x1b), the error is permanent and
> retrying will never succeed. Previously, only EBADFD was treated as
> fatal, causing server_cb to return TRUE for EOPNOTSUPP. Since the fd
> remains readable, this creates an infinite busy loop that hangs
> bluetoothd.
>
> [...]
Here is the summary with links:
- [BlueZ,v2,1/4] btio: Handle EOPNOTSUPP from accept() to prevent busy loop
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8c580d30d265
- [BlueZ,v2,2/4] profile: Check if bearer is enabled on registration
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=abf0911e25ef
- [BlueZ,v2,3/4] plugins: Check btd_profile_register return value
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=7b5895d80635
- [BlueZ,v2,4/4] bearer: Check btd_opts.mode on btd_bearer_new
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8c821a150b8f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* RE: [v2] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: bluez.test.bot @ 2026-06-10 18:28 UTC (permalink / raw)
To: linux-bluetooth, oss
In-Reply-To: <20260610150704.1234558-1-oss@fourdim.xyz>
[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1109363
---Test result---
Test Summary:
CheckPatch PASS 1.14 seconds
VerifyFixes PASS 0.11 seconds
VerifySignedoff PASS 0.11 seconds
GitLint PASS 0.29 seconds
SubjectPrefix PASS 0.11 seconds
BuildKernel PASS 27.22 seconds
CheckAllWarning PASS 29.76 seconds
CheckSparse PASS 28.66 seconds
BuildKernel32 PASS 26.45 seconds
TestRunnerSetup PASS 579.03 seconds
TestRunner_l2cap-tester PASS 60.13 seconds
TestRunner_iso-tester PASS 74.99 seconds
TestRunner_bnep-tester PASS 19.60 seconds
TestRunner_mgmt-tester FAIL 214.34 seconds
TestRunner_rfcomm-tester PASS 25.55 seconds
TestRunner_sco-tester PASS 32.90 seconds
TestRunner_ioctl-tester PASS 26.59 seconds
TestRunner_mesh-tester FAIL 26.13 seconds
TestRunner_smp-tester PASS 23.46 seconds
TestRunner_userchan-tester PASS 20.07 seconds
TestRunner_6lowpan-tester PASS 23.57 seconds
IncrementalBuild PASS 26.07 seconds
Details
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4
Failed Test Cases
Read Exp Feature - Success Failed 0.248 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0
Failed Test Cases
Mesh - Send cancel - 1 Timed out 2.228 seconds
Mesh - Send cancel - 2 Timed out 1.988 seconds
https://github.com/bluez/bluetooth-next/pull/302
---
Regards,
Linux Bluetooth
^ permalink raw reply
* Re: [PATCH BlueZ 0/2] btio: handle error from broadcast ISO socket
From: patchwork-bot+bluetooth @ 2026-06-10 18:20 UTC (permalink / raw)
To: Michal Dzik; +Cc: linux-bluetooth
In-Reply-To: <20260606060348.2240980-1-michal.dzik@streamunlimited.com>
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Sat, 6 Jun 2026 08:03:46 +0200 you wrote:
> If not handled, server_cb() will be stuck in endless loop of calling
> accept() on the socket.
>
> Michal Dzik (2):
> btio: handle error from broadcast ISO socket
> bap: log errors in BIG sync
>
> [...]
Here is the summary with links:
- [BlueZ,1/2] btio: handle error from broadcast ISO socket
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ea4b494414f6
- [BlueZ,2/2] bap: log errors in BIG sync
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6ea27ab0dcfc
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [GIT PULL] bluetooth 2026-06-10
From: Luiz Augusto von Dentz @ 2026-06-10 18:00 UTC (permalink / raw)
To: davem, kuba; +Cc: linux-bluetooth, netdev
The following changes since commit 149324fc762c2a7acef9c26790566f81f475e51f:
Bluetooth: MGMT: Fix backward compatibility with userspace (2026-06-03 11:24:12 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git tags/for-net-2026-06-10
for you to fetch changes up to f98220e190507d3eb3e02f9a2f20c418c30d04ae:
Bluetooth: vhci: validate devcoredump state before side effects (2026-06-10 13:51:15 -0400)
----------------------------------------------------------------
bluetooth pull request for net:
- hci_core: Fix UAF in hci_unregister_dev()
- hci_event: fix simultaneous discovery stuck in FINDING
- hci: validate codec capability element length
- eir: Fix stack OOB write when prepending the Flags AD
- L2CAP: Fix UAF in channel timeout by holding conn ref
- btusb: fix use-after-free on registration failure
- btusb: fix use-after-free on marvell probe failure
- btusb: fix wakeup source leak on probe failure
- btusb: fix wakeup irq devres lifetime
- btmtk: fix URB leak in alloc_mtk_intr_urb error path
- vhci: validate devcoredump state before side effects
----------------------------------------------------------------
Jiajia Liu (1):
Bluetooth: hci_event: fix simultaneous discovery stuck in FINDING
Johan Hovold (4):
Bluetooth: btusb: fix use-after-free on registration failure
Bluetooth: btusb: fix use-after-free on marvell probe failure
Bluetooth: btusb: fix wakeup source leak on probe failure
Bluetooth: btusb: fix wakeup irq devres lifetime
Jordan Walters (1):
Bluetooth: hci_core: Fix UAF in hci_unregister_dev()
Marco Elver (1):
Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref
Samuel Moelius (2):
Bluetooth: hci: validate codec capability element length
Bluetooth: vhci: validate devcoredump state before side effects
Weiming Shi (1):
Bluetooth: eir: Fix stack OOB write when prepending the Flags AD
Zhao Dongdong (1):
Bluetooth: btmtk: fix URB leak in alloc_mtk_intr_urb error path
drivers/bluetooth/btmtk.c | 4 +++-
drivers/bluetooth/btusb.c | 55 ++++++++++++++++++++++++++++++++++---------
drivers/bluetooth/hci_vhci.c | 10 ++++++++
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/eir.c | 8 ++++---
net/bluetooth/hci_codec.c | 2 +-
net/bluetooth/hci_core.c | 2 ++
net/bluetooth/hci_event.c | 7 ++++++
net/bluetooth/l2cap_core.c | 34 +++++++++++++++-----------
9 files changed, 93 insertions(+), 30 deletions(-)
^ permalink raw reply
* RE: [v2] Bluetooth: btintel_pcie: Separate coredump work from RX work
From: bluez.test.bot @ 2026-06-10 17:53 UTC (permalink / raw)
To: linux-bluetooth, kiran.k
In-Reply-To: <20260610162544.240444-1-kiran.k@intel.com>
[-- Attachment #1: Type: text/plain, Size: 567 bytes --]
This is an automated email and please do not reply to this email.
Dear Submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.
----- Output -----
error: patch failed: drivers/bluetooth/btintel_pcie.c:1457
error: drivers/bluetooth/btintel_pcie.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch
Please resolve the issue and submit the patches again.
---
Regards,
Linux Bluetooth
^ permalink raw reply
* Re: [PATCH v9 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()
From: Siwei Zhang @ 2026-06-10 17:13 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <CABBYNZKuKSs938J6Pt_gqJzj4s1vz3wTGzJsfowS7H_n6mPd4A@mail.gmail.com>
On Wed, Jun 10, 2026, at 1:12 PM, Luiz Augusto von Dentz wrote:
> Hi Siwei,
>
> On Wed, Jun 10, 2026 at 1:03 PM Siwei Zhang <oss@fourdim.xyz> wrote:
>>
>>
>> On Wed, Jun 10, 2026, at 12:59 PM, Luiz Augusto von Dentz wrote:
>> > Hi Siwei,
>> >
>> > On Wed, Jun 10, 2026 at 12:33 PM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >>
>> >>
>> >>
>> >> On Wed, Jun 10, 2026, at 11:57 AM, Luiz Augusto von Dentz wrote:
>> >> > Hi Siwei,
>> >> >
>> >> > On Mon, Jun 8, 2026 at 9:29 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >> >>
>> >> >> Hi Luiz,
>> >> >>
>> >> >> On Thu, Jun 4, 2026, at 11:52 AM, Siwei Zhang wrote:
>> >> >> > Hi Luiz,
>> >> >> >
>> >> >> > On Wed, Jun 3, 2026, at 2:17 PM, Luiz Augusto von Dentz wrote:
>> >> >> >> Hi Siwei,
>> >> >> >>
>> >> >> >> On Wed, Jun 3, 2026 at 11:09 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >> >> >>>
>> >> >> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
>> >> >> >>> release_sock(parent). Once the parent lock is released, the child
>> >> >> >>> socket sk can be freed by another task.
>> >> >> >>>
>> >> >> >>> Allocate the channel outside the func to prevent this.
>> >> >> >>>
>> >> >> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
>> >> >> >>> Cc: stable@kernel.org
>> >> >> >>> Assisted-by: Claude:claude-opus-4-8
>> >> >> >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
>> >> >> >>> ---
>> >> >> >>> include/net/bluetooth/l2cap.h | 10 +++--
>> >> >> >>> net/bluetooth/6lowpan.c | 31 +++++++------
>> >> >> >>> net/bluetooth/l2cap_core.c | 41 ++++++++++++-----
>> >> >> >>> net/bluetooth/l2cap_sock.c | 83 +++++++++++++++++++++--------------
>> >> >> >>> net/bluetooth/smp.c | 17 ++++---
>> >> >> >>> 5 files changed, 113 insertions(+), 69 deletions(-)
>> >> >> >>>
>> >> >> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> >> >> >>> index e0a1f2293679..7f5e4647f6e0 100644
>> >> >> >>> --- a/include/net/bluetooth/l2cap.h
>> >> >> >>> +++ b/include/net/bluetooth/l2cap.h
>> >> >> >>> @@ -620,7 +620,9 @@ struct l2cap_chan {
>> >> >> >>> struct l2cap_ops {
>> >> >> >>> char *name;
>> >> >> >>>
>> >> >> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
>> >> >> >>> + int (*new_connection)(struct l2cap_conn *conn,
>> >> >> >>> + struct l2cap_chan *chan,
>> >> >> >>> + struct l2cap_chan *new_chan);
>> >> >> >>> int (*recv) (struct l2cap_chan * chan,
>> >> >> >>> struct sk_buff *skb);
>> >> >> >>> void (*teardown) (struct l2cap_chan *chan, int err);
>> >> >> >>> @@ -884,9 +886,11 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
>> >> >> >>> return (seq + 1) % (chan->tx_win_max + 1);
>> >> >> >>> }
>> >> >> >>>
>> >> >> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
>> >> >> >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_conn *conn,
>> >> >> >>> + struct l2cap_chan *chan,
>> >> >> >>> + struct l2cap_chan *new_chan)
>> >> >> >>> {
>> >> >> >>> - return NULL;
>> >> >> >>> + return -EOPNOTSUPP;
>> >> >> >>> }
>> >> >> >>>
>> >> >> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb)
>> >> >> >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> >> >> >>> index cb1e329d66fd..94863af97a44 100644
>> >> >> >>> --- a/net/bluetooth/6lowpan.c
>> >> >> >>> +++ b/net/bluetooth/6lowpan.c
>> >> >> >>> @@ -624,6 +624,15 @@ static bool is_bt_6lowpan(struct hci_conn *hcon)
>> >> >> >>> return true;
>> >> >> >>> }
>> >> >> >>>
>> >> >> >>> +static void chan_init(struct l2cap_chan *chan)
>> >> >> >>> +{
>> >> >> >>> + l2cap_chan_set_defaults(chan);
>> >> >> >>> +
>> >> >> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> >> >> >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> >> >> >>> + chan->imtu = 1280;
>> >> >> >>> +}
>> >> >> >>> +
>> >> >> >>> static struct l2cap_chan *chan_create(void)
>> >> >> >>> {
>> >> >> >>> struct l2cap_chan *chan;
>> >> >> >>> @@ -632,11 +641,7 @@ static struct l2cap_chan *chan_create(void)
>> >> >> >>> if (!chan)
>> >> >> >>> return NULL;
>> >> >> >>>
>> >> >> >>> - l2cap_chan_set_defaults(chan);
>> >> >> >>> -
>> >> >> >>> - chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> >> >> >>> - chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> >> >> >>> - chan->imtu = 1280;
>> >> >> >>> + chan_init(chan);
>> >> >> >>>
>> >> >> >>> return chan;
>> >> >> >>> }
>> >> >> >>> @@ -745,19 +750,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan)
>> >> >> >>> ifup(dev->netdev);
>> >> >> >>> }
>> >> >> >>>
>> >> >> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
>> >> >> >>> +static inline int chan_new_conn_cb(struct l2cap_conn *conn,
>> >> >> >>> + struct l2cap_chan *pchan,
>> >> >> >>> + struct l2cap_chan *chan)
>> >> >> >>> {
>> >> >> >>> - struct l2cap_chan *chan;
>> >> >> >>> -
>> >> >> >>> - chan = chan_create();
>> >> >> >>> - if (!chan)
>> >> >> >>> - return NULL;
>> >> >> >>> -
>> >> >> >>> + chan_init(chan);
>> >>
>> >> ref1: chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; set
>> >> in chan_init.
>> >>
>> >> >> >>> chan->ops = pchan->ops;
>> >> >> >>>
>> >> >> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
>> >> >> >>> + __l2cap_chan_add(conn, chan);
>> >> >> >>> +
>> >> >> >>> BT_DBG("chan %p pchan %p", chan, pchan);
>> >> >> >>>
>> >> >> >>> - return chan;
>> >> >> >>> + return 0;
>> >> >> >>> }
>> >> >> >>>
>> >> >> >>> static void unregister_dev(struct lowpan_btle_dev *dev)
>> >> >> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> >> >> >>> index c4ccfbda9d78..62acf90837fb 100644
>> >> >> >>> --- a/net/bluetooth/l2cap_core.c
>> >> >> >>> +++ b/net/bluetooth/l2cap_core.c
>> >> >> >>> @@ -4007,6 +4007,31 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
>> >> >> >>> return 0;
>> >> >> >>> }
>> >> >> >>>
>> >> >> >>> +/* Allocate and initialise a channel for an incoming connection.
>> >> >> >>> + *
>> >> >> >>> + * ->new_connection() initialises the channel and links it into @conn with
>> >> >> >>> + * __l2cap_chan_add(). The l2cap_chan_create() reference becomes the one owned
>> >> >> >>> + * by the parent subsystem (l2cap_pi(sk)->chan, conn->smp or peer->chan) and is
>> >> >> >>> + * released by its teardown callback; the conn list reference is released by
>> >> >> >>> + * l2cap_chan_del().
>> >> >> >>> + */
>> >> >> >>> +static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
>> >> >> >>> + struct l2cap_chan *pchan)
>> >> >> >>> +{
>> >> >> >>> + struct l2cap_chan *chan;
>> >> >> >>> +
>> >> >> >>> + chan = l2cap_chan_create();
>> >> >> >>> + if (!chan)
>> >> >> >>> + return NULL;
>> >> >> >>> +
>> >> >> >>> + if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
>> >> >> >>> + l2cap_chan_put(chan);
>> >> >> >>> + return NULL;
>> >> >> >>> + }
>> >> >> >>
>> >> >> >> I don't quite get why we can't just place __l2cap_chan_add here
>> >> >> >> instead of having it called by new_connection callbacks?
>> >> >> >>
>> >> >> >
>> >> >> > It's specifically the l2cap_sock_new_connection_cb() case - the very
>> >> >> > use-after-free this patch fixes. The __l2cap_chan_add() has to happen while
>> >> >> > the parent lock is still held, and only the callback holds that lock.
>> >> >> >
>> >> >> > The reference counting on the new child chan starts at one ref,
>> >> >> > owned by the new socket:
>> >> >> >
>> >> >> > /* l2cap_new_connection() */
>> >> >> > chan = l2cap_chan_create(); /* refcount = 1 */
>> >> >> > if (!chan)
>> >> >> > return NULL;
>> >> >> >
>> >> >> > pchan->ops->new_connection(conn, pchan, chan);
>> >> >> >
>> >> >> > and inside the socket callback:
>> >> >> >
>> >> >> > /* l2cap_sock_new_connection_cb() */
>> >> >> > lock_sock(parent);
>> >> >> > ...
>> >> >> > sk = l2cap_sock_alloc(..., new_chan); /* sk owns the chan_create ref */
>> >> >> > ...
>> >> >> > l2cap_sock_init(sk, parent);
>> >> >> >
>> >> >> > __l2cap_chan_add(conn, new_chan); /* (A) conn list takes a ref */
>> >> >> > bt_accept_enqueue(parent, sk, false); /* (B) sk now on accept queue */
>> >> >> >
>> >> >> > release_sock(parent); /* (C) parent lock dropped */
>> >> >> > return 0;
>> >> >> >
>> >> >> > The moment we hit (C), sk is reachable through the parent's accept queue, so
>> >> >> > another task can grab and tear it down:
>> >> >> >
>> >> >> > accept() -> l2cap_sock_kill() -> l2cap_sock_put_chan()
>> >> >> > chan->data = NULL;
>> >> >> > l2cap_chan_put(chan); /* drops the sk's chan ref */
>> >> >> >
>> >> >> > If __l2cap_chan_add() at (A) hadn't already taken the conn list reference,
>> >> >> > that put would drop the last ref and free new_chan. Control then returns up
>> >> >> > to l2cap_new_connection(), which hands the now-freed chan back to
>> >> >> > l2cap_connect():
>> >> >> >
>> >> >> > /* l2cap_connect() - runs after the callback returns */
>> >> >> > chan = l2cap_new_connection(conn, pchan);
>> >> >> > if (!chan)
>> >> >> > goto response;
>> >> >> > ...
>> >> >> > bacpy(&chan->src, &conn->hcon->src); /* <-- UAF on freed chan */
>> >> >> > chan->psm = psm;
>> >> >> > chan->dcid = scid;
>> >> >> >
>> >> >> > The conn list reference taken at (A), before (C), is what keeps new_chan
>> >> >> > alive across the release_sock() window so l2cap_connect() can keep using it.
>> >> >> >
>> >> >> > So __l2cap_chan_add() can't move out to l2cap_new_connection(): by the time
>> >> >> > the callback returns, the parent lock is already dropped and chan may already
>> >> >> > be freed - which is exactly the race. It has to be taken inside the callback,
>> >> >> > under the parent lock, before the socket is exposed.
>> >> >
>> >> > If you do after l2cap_new_connection, yes, but what if you do before
>> >> > you call ->new_connect:
>> >> >
>> >> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> >> > index 3991c179b98f..cd864f400985 100644
>> >> > --- a/net/bluetooth/l2cap_core.c
>> >> > +++ b/net/bluetooth/l2cap_core.c
>> >> > @@ -4027,8 +4027,10 @@ static struct l2cap_chan
>> >> > *l2cap_new_connection(struct l2cap_conn *conn,
>> >> > if (!chan)
>> >> > return NULL;
>> >> >
>> >> > + __l2cap_chan_add(conn, chan);
>> >> > +
>> >> > if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
>> >> > - l2cap_chan_put(chan);
>> >> > + l2cap_chan_del(chan, 0);
>> >> > return NULL;
>> >> > }
>> >> >
>> >> > This means we have to call l2cap_chan_del if new_connection fails, but
>> >> > we can eliminate other functions having to call __l2cap_chan_add with
>> >> > this. If the issue is then having l2cap_chan_del called concurrently,
>> >> > we can add a l2cap_chan_hold; actually, we may need a reference anyway
>> >> > since sk can be released and call l2cap_chan_del, releasing both
>> >> > references.
>> >> >
>> >>
>> >> In:
>> >>
>> >> void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
>> >> {
>> >> ...
>> >> switch (chan->chan_type) {
>> >> case L2CAP_CHAN_CONN_ORIENTED:
>> >> ...
>> >> break;
>> >> case L2CAP_CHAN_CONN_LESS:
>> >> ...
>> >> break;
>> >> case L2CAP_CHAN_FIXED:
>> >> ...
>> >> break;
>> >> default:
>> >> ...
>> >> }
>> >> }
>> >>
>> >> Apparently it depends on chan->chan_type and it is set by ref1.
>> >> It is only set in cb function.
>> >
>> > Ok, this probably needs fixing in 6lowpan. These defaults should come
>> > from the parent channel, similar to how it's done in l2cap_sock_init.
>> > Perhaps we should promote that logic to l2cap_chan_set_defaults
>> > passing pchan so it can initialize the channel before calling
>> > __l2cap_chan_add.
>>
>> I can fix that in a separate patch.
>> Could you please merge it first?
>
> Merge it first, and then rework the order? Not sure what is the
> benefit of doing that, it is a somewhat substatial change either way
> so I don't think it will make it much easier to lets say backport.
>
Will do a patch v10.
>> Best,
>> Siwei
>
>
>
> --
> Luiz Augusto von Dentz
Best,
Siwei
^ permalink raw reply
* Re: [PATCH] Bluetooth: hci_bcm4377: Use named initializers for pci_device_id array
From: Luiz Augusto von Dentz @ 2026-06-10 17:13 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Sven Peter, Janne Grunau, Neal Gompa, Marcel Holtmann,
Markus Schneider-Pargmann, asahi, linux-arm-kernel,
linux-bluetooth, linux-kernel
In-Reply-To: <aimXkX4FC4JVxt6q@monoceros>
Hi Uwe,
On Wed, Jun 10, 2026 at 12:59 PM Uwe Kleine-König (The Capable Hub)
<u.kleine-koenig@baylibre.com> wrote:
>
> On Mon, May 04, 2026 at 06:09:40PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > Initializing a struct using list initializers is hard to read, compared
> > to that using named initializers is more ideomatic. Convert the macro
> > used to assign values in the driver's pci_device_id array accordingly.
> >
> > This change doesn't introduce any changes to the compiled array on an
> > x86 and an arm64 build.
> >
> > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> > ---
> > Hello,
> >
> > this is a preparing change for making struct pci_device_id::driver_data an
> > anonymous union (similar to
> > https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/).
> > This requires named initializers for .driver_data. But even without that
> > this is a nice cleanup making the macro better readable.
> >
> > Gcc is happy with simplifying the assignment further using
> > PCI_VDEVICE(BROADCOM, BCM ## id ## _DEVICE_ID), but this is a bit fishy
> > because PCI_VDEVICE also assigns .class and .class_mask (using list
> > initializers), so I didn't convert that.
>
> In the meantime I learned that doing that would break W=1 builds, so it
> was a good choice to not go that path.
>
> > Once all pci_device_id use
> > named initializers, the two zeros can be dropped from PCI_VDEVICE and
> > this entry simplified accordingly.
>
> Is this patch still on someone's radar? Ideally for application in time
> for 7.2-rc1?
It is no longer in patchwork so if you really want to get in please resend.
> Best regards
> Uwe
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v9 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()
From: Luiz Augusto von Dentz @ 2026-06-10 17:12 UTC (permalink / raw)
To: Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <e9720437-0673-4428-9be2-203404e5cbec@app.fastmail.com>
Hi Siwei,
On Wed, Jun 10, 2026 at 1:03 PM Siwei Zhang <oss@fourdim.xyz> wrote:
>
>
> On Wed, Jun 10, 2026, at 12:59 PM, Luiz Augusto von Dentz wrote:
> > Hi Siwei,
> >
> > On Wed, Jun 10, 2026 at 12:33 PM Siwei Zhang <oss@fourdim.xyz> wrote:
> >>
> >>
> >>
> >> On Wed, Jun 10, 2026, at 11:57 AM, Luiz Augusto von Dentz wrote:
> >> > Hi Siwei,
> >> >
> >> > On Mon, Jun 8, 2026 at 9:29 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >> >>
> >> >> Hi Luiz,
> >> >>
> >> >> On Thu, Jun 4, 2026, at 11:52 AM, Siwei Zhang wrote:
> >> >> > Hi Luiz,
> >> >> >
> >> >> > On Wed, Jun 3, 2026, at 2:17 PM, Luiz Augusto von Dentz wrote:
> >> >> >> Hi Siwei,
> >> >> >>
> >> >> >> On Wed, Jun 3, 2026 at 11:09 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >> >> >>>
> >> >> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
> >> >> >>> release_sock(parent). Once the parent lock is released, the child
> >> >> >>> socket sk can be freed by another task.
> >> >> >>>
> >> >> >>> Allocate the channel outside the func to prevent this.
> >> >> >>>
> >> >> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
> >> >> >>> Cc: stable@kernel.org
> >> >> >>> Assisted-by: Claude:claude-opus-4-8
> >> >> >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> >> >> >>> ---
> >> >> >>> include/net/bluetooth/l2cap.h | 10 +++--
> >> >> >>> net/bluetooth/6lowpan.c | 31 +++++++------
> >> >> >>> net/bluetooth/l2cap_core.c | 41 ++++++++++++-----
> >> >> >>> net/bluetooth/l2cap_sock.c | 83 +++++++++++++++++++++--------------
> >> >> >>> net/bluetooth/smp.c | 17 ++++---
> >> >> >>> 5 files changed, 113 insertions(+), 69 deletions(-)
> >> >> >>>
> >> >> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >> >> >>> index e0a1f2293679..7f5e4647f6e0 100644
> >> >> >>> --- a/include/net/bluetooth/l2cap.h
> >> >> >>> +++ b/include/net/bluetooth/l2cap.h
> >> >> >>> @@ -620,7 +620,9 @@ struct l2cap_chan {
> >> >> >>> struct l2cap_ops {
> >> >> >>> char *name;
> >> >> >>>
> >> >> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
> >> >> >>> + int (*new_connection)(struct l2cap_conn *conn,
> >> >> >>> + struct l2cap_chan *chan,
> >> >> >>> + struct l2cap_chan *new_chan);
> >> >> >>> int (*recv) (struct l2cap_chan * chan,
> >> >> >>> struct sk_buff *skb);
> >> >> >>> void (*teardown) (struct l2cap_chan *chan, int err);
> >> >> >>> @@ -884,9 +886,11 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
> >> >> >>> return (seq + 1) % (chan->tx_win_max + 1);
> >> >> >>> }
> >> >> >>>
> >> >> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> >> >> >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_conn *conn,
> >> >> >>> + struct l2cap_chan *chan,
> >> >> >>> + struct l2cap_chan *new_chan)
> >> >> >>> {
> >> >> >>> - return NULL;
> >> >> >>> + return -EOPNOTSUPP;
> >> >> >>> }
> >> >> >>>
> >> >> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb)
> >> >> >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> >> >> >>> index cb1e329d66fd..94863af97a44 100644
> >> >> >>> --- a/net/bluetooth/6lowpan.c
> >> >> >>> +++ b/net/bluetooth/6lowpan.c
> >> >> >>> @@ -624,6 +624,15 @@ static bool is_bt_6lowpan(struct hci_conn *hcon)
> >> >> >>> return true;
> >> >> >>> }
> >> >> >>>
> >> >> >>> +static void chan_init(struct l2cap_chan *chan)
> >> >> >>> +{
> >> >> >>> + l2cap_chan_set_defaults(chan);
> >> >> >>> +
> >> >> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> >> >> >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >> >> >>> + chan->imtu = 1280;
> >> >> >>> +}
> >> >> >>> +
> >> >> >>> static struct l2cap_chan *chan_create(void)
> >> >> >>> {
> >> >> >>> struct l2cap_chan *chan;
> >> >> >>> @@ -632,11 +641,7 @@ static struct l2cap_chan *chan_create(void)
> >> >> >>> if (!chan)
> >> >> >>> return NULL;
> >> >> >>>
> >> >> >>> - l2cap_chan_set_defaults(chan);
> >> >> >>> -
> >> >> >>> - chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> >> >> >>> - chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >> >> >>> - chan->imtu = 1280;
> >> >> >>> + chan_init(chan);
> >> >> >>>
> >> >> >>> return chan;
> >> >> >>> }
> >> >> >>> @@ -745,19 +750,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan)
> >> >> >>> ifup(dev->netdev);
> >> >> >>> }
> >> >> >>>
> >> >> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
> >> >> >>> +static inline int chan_new_conn_cb(struct l2cap_conn *conn,
> >> >> >>> + struct l2cap_chan *pchan,
> >> >> >>> + struct l2cap_chan *chan)
> >> >> >>> {
> >> >> >>> - struct l2cap_chan *chan;
> >> >> >>> -
> >> >> >>> - chan = chan_create();
> >> >> >>> - if (!chan)
> >> >> >>> - return NULL;
> >> >> >>> -
> >> >> >>> + chan_init(chan);
> >>
> >> ref1: chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; set
> >> in chan_init.
> >>
> >> >> >>> chan->ops = pchan->ops;
> >> >> >>>
> >> >> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
> >> >> >>> + __l2cap_chan_add(conn, chan);
> >> >> >>> +
> >> >> >>> BT_DBG("chan %p pchan %p", chan, pchan);
> >> >> >>>
> >> >> >>> - return chan;
> >> >> >>> + return 0;
> >> >> >>> }
> >> >> >>>
> >> >> >>> static void unregister_dev(struct lowpan_btle_dev *dev)
> >> >> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> >> >>> index c4ccfbda9d78..62acf90837fb 100644
> >> >> >>> --- a/net/bluetooth/l2cap_core.c
> >> >> >>> +++ b/net/bluetooth/l2cap_core.c
> >> >> >>> @@ -4007,6 +4007,31 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
> >> >> >>> return 0;
> >> >> >>> }
> >> >> >>>
> >> >> >>> +/* Allocate and initialise a channel for an incoming connection.
> >> >> >>> + *
> >> >> >>> + * ->new_connection() initialises the channel and links it into @conn with
> >> >> >>> + * __l2cap_chan_add(). The l2cap_chan_create() reference becomes the one owned
> >> >> >>> + * by the parent subsystem (l2cap_pi(sk)->chan, conn->smp or peer->chan) and is
> >> >> >>> + * released by its teardown callback; the conn list reference is released by
> >> >> >>> + * l2cap_chan_del().
> >> >> >>> + */
> >> >> >>> +static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
> >> >> >>> + struct l2cap_chan *pchan)
> >> >> >>> +{
> >> >> >>> + struct l2cap_chan *chan;
> >> >> >>> +
> >> >> >>> + chan = l2cap_chan_create();
> >> >> >>> + if (!chan)
> >> >> >>> + return NULL;
> >> >> >>> +
> >> >> >>> + if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
> >> >> >>> + l2cap_chan_put(chan);
> >> >> >>> + return NULL;
> >> >> >>> + }
> >> >> >>
> >> >> >> I don't quite get why we can't just place __l2cap_chan_add here
> >> >> >> instead of having it called by new_connection callbacks?
> >> >> >>
> >> >> >
> >> >> > It's specifically the l2cap_sock_new_connection_cb() case - the very
> >> >> > use-after-free this patch fixes. The __l2cap_chan_add() has to happen while
> >> >> > the parent lock is still held, and only the callback holds that lock.
> >> >> >
> >> >> > The reference counting on the new child chan starts at one ref,
> >> >> > owned by the new socket:
> >> >> >
> >> >> > /* l2cap_new_connection() */
> >> >> > chan = l2cap_chan_create(); /* refcount = 1 */
> >> >> > if (!chan)
> >> >> > return NULL;
> >> >> >
> >> >> > pchan->ops->new_connection(conn, pchan, chan);
> >> >> >
> >> >> > and inside the socket callback:
> >> >> >
> >> >> > /* l2cap_sock_new_connection_cb() */
> >> >> > lock_sock(parent);
> >> >> > ...
> >> >> > sk = l2cap_sock_alloc(..., new_chan); /* sk owns the chan_create ref */
> >> >> > ...
> >> >> > l2cap_sock_init(sk, parent);
> >> >> >
> >> >> > __l2cap_chan_add(conn, new_chan); /* (A) conn list takes a ref */
> >> >> > bt_accept_enqueue(parent, sk, false); /* (B) sk now on accept queue */
> >> >> >
> >> >> > release_sock(parent); /* (C) parent lock dropped */
> >> >> > return 0;
> >> >> >
> >> >> > The moment we hit (C), sk is reachable through the parent's accept queue, so
> >> >> > another task can grab and tear it down:
> >> >> >
> >> >> > accept() -> l2cap_sock_kill() -> l2cap_sock_put_chan()
> >> >> > chan->data = NULL;
> >> >> > l2cap_chan_put(chan); /* drops the sk's chan ref */
> >> >> >
> >> >> > If __l2cap_chan_add() at (A) hadn't already taken the conn list reference,
> >> >> > that put would drop the last ref and free new_chan. Control then returns up
> >> >> > to l2cap_new_connection(), which hands the now-freed chan back to
> >> >> > l2cap_connect():
> >> >> >
> >> >> > /* l2cap_connect() - runs after the callback returns */
> >> >> > chan = l2cap_new_connection(conn, pchan);
> >> >> > if (!chan)
> >> >> > goto response;
> >> >> > ...
> >> >> > bacpy(&chan->src, &conn->hcon->src); /* <-- UAF on freed chan */
> >> >> > chan->psm = psm;
> >> >> > chan->dcid = scid;
> >> >> >
> >> >> > The conn list reference taken at (A), before (C), is what keeps new_chan
> >> >> > alive across the release_sock() window so l2cap_connect() can keep using it.
> >> >> >
> >> >> > So __l2cap_chan_add() can't move out to l2cap_new_connection(): by the time
> >> >> > the callback returns, the parent lock is already dropped and chan may already
> >> >> > be freed - which is exactly the race. It has to be taken inside the callback,
> >> >> > under the parent lock, before the socket is exposed.
> >> >
> >> > If you do after l2cap_new_connection, yes, but what if you do before
> >> > you call ->new_connect:
> >> >
> >> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> > index 3991c179b98f..cd864f400985 100644
> >> > --- a/net/bluetooth/l2cap_core.c
> >> > +++ b/net/bluetooth/l2cap_core.c
> >> > @@ -4027,8 +4027,10 @@ static struct l2cap_chan
> >> > *l2cap_new_connection(struct l2cap_conn *conn,
> >> > if (!chan)
> >> > return NULL;
> >> >
> >> > + __l2cap_chan_add(conn, chan);
> >> > +
> >> > if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
> >> > - l2cap_chan_put(chan);
> >> > + l2cap_chan_del(chan, 0);
> >> > return NULL;
> >> > }
> >> >
> >> > This means we have to call l2cap_chan_del if new_connection fails, but
> >> > we can eliminate other functions having to call __l2cap_chan_add with
> >> > this. If the issue is then having l2cap_chan_del called concurrently,
> >> > we can add a l2cap_chan_hold; actually, we may need a reference anyway
> >> > since sk can be released and call l2cap_chan_del, releasing both
> >> > references.
> >> >
> >>
> >> In:
> >>
> >> void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> >> {
> >> ...
> >> switch (chan->chan_type) {
> >> case L2CAP_CHAN_CONN_ORIENTED:
> >> ...
> >> break;
> >> case L2CAP_CHAN_CONN_LESS:
> >> ...
> >> break;
> >> case L2CAP_CHAN_FIXED:
> >> ...
> >> break;
> >> default:
> >> ...
> >> }
> >> }
> >>
> >> Apparently it depends on chan->chan_type and it is set by ref1.
> >> It is only set in cb function.
> >
> > Ok, this probably needs fixing in 6lowpan. These defaults should come
> > from the parent channel, similar to how it's done in l2cap_sock_init.
> > Perhaps we should promote that logic to l2cap_chan_set_defaults
> > passing pchan so it can initialize the channel before calling
> > __l2cap_chan_add.
>
> I can fix that in a separate patch.
> Could you please merge it first?
Merge it first, and then rework the order? Not sure what is the
benefit of doing that, it is a somewhat substatial change either way
so I don't think it will make it much easier to lets say backport.
> Best,
> Siwei
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v9 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()
From: Siwei Zhang @ 2026-06-10 17:02 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <CABBYNZJsyPU5wr4T32Ej9Vj0=Nco=PjtQn4aiC=NqUGbgP3zBw@mail.gmail.com>
On Wed, Jun 10, 2026, at 12:59 PM, Luiz Augusto von Dentz wrote:
> Hi Siwei,
>
> On Wed, Jun 10, 2026 at 12:33 PM Siwei Zhang <oss@fourdim.xyz> wrote:
>>
>>
>>
>> On Wed, Jun 10, 2026, at 11:57 AM, Luiz Augusto von Dentz wrote:
>> > Hi Siwei,
>> >
>> > On Mon, Jun 8, 2026 at 9:29 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >>
>> >> Hi Luiz,
>> >>
>> >> On Thu, Jun 4, 2026, at 11:52 AM, Siwei Zhang wrote:
>> >> > Hi Luiz,
>> >> >
>> >> > On Wed, Jun 3, 2026, at 2:17 PM, Luiz Augusto von Dentz wrote:
>> >> >> Hi Siwei,
>> >> >>
>> >> >> On Wed, Jun 3, 2026 at 11:09 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >> >>>
>> >> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
>> >> >>> release_sock(parent). Once the parent lock is released, the child
>> >> >>> socket sk can be freed by another task.
>> >> >>>
>> >> >>> Allocate the channel outside the func to prevent this.
>> >> >>>
>> >> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
>> >> >>> Cc: stable@kernel.org
>> >> >>> Assisted-by: Claude:claude-opus-4-8
>> >> >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
>> >> >>> ---
>> >> >>> include/net/bluetooth/l2cap.h | 10 +++--
>> >> >>> net/bluetooth/6lowpan.c | 31 +++++++------
>> >> >>> net/bluetooth/l2cap_core.c | 41 ++++++++++++-----
>> >> >>> net/bluetooth/l2cap_sock.c | 83 +++++++++++++++++++++--------------
>> >> >>> net/bluetooth/smp.c | 17 ++++---
>> >> >>> 5 files changed, 113 insertions(+), 69 deletions(-)
>> >> >>>
>> >> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> >> >>> index e0a1f2293679..7f5e4647f6e0 100644
>> >> >>> --- a/include/net/bluetooth/l2cap.h
>> >> >>> +++ b/include/net/bluetooth/l2cap.h
>> >> >>> @@ -620,7 +620,9 @@ struct l2cap_chan {
>> >> >>> struct l2cap_ops {
>> >> >>> char *name;
>> >> >>>
>> >> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
>> >> >>> + int (*new_connection)(struct l2cap_conn *conn,
>> >> >>> + struct l2cap_chan *chan,
>> >> >>> + struct l2cap_chan *new_chan);
>> >> >>> int (*recv) (struct l2cap_chan * chan,
>> >> >>> struct sk_buff *skb);
>> >> >>> void (*teardown) (struct l2cap_chan *chan, int err);
>> >> >>> @@ -884,9 +886,11 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
>> >> >>> return (seq + 1) % (chan->tx_win_max + 1);
>> >> >>> }
>> >> >>>
>> >> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
>> >> >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_conn *conn,
>> >> >>> + struct l2cap_chan *chan,
>> >> >>> + struct l2cap_chan *new_chan)
>> >> >>> {
>> >> >>> - return NULL;
>> >> >>> + return -EOPNOTSUPP;
>> >> >>> }
>> >> >>>
>> >> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb)
>> >> >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> >> >>> index cb1e329d66fd..94863af97a44 100644
>> >> >>> --- a/net/bluetooth/6lowpan.c
>> >> >>> +++ b/net/bluetooth/6lowpan.c
>> >> >>> @@ -624,6 +624,15 @@ static bool is_bt_6lowpan(struct hci_conn *hcon)
>> >> >>> return true;
>> >> >>> }
>> >> >>>
>> >> >>> +static void chan_init(struct l2cap_chan *chan)
>> >> >>> +{
>> >> >>> + l2cap_chan_set_defaults(chan);
>> >> >>> +
>> >> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> >> >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> >> >>> + chan->imtu = 1280;
>> >> >>> +}
>> >> >>> +
>> >> >>> static struct l2cap_chan *chan_create(void)
>> >> >>> {
>> >> >>> struct l2cap_chan *chan;
>> >> >>> @@ -632,11 +641,7 @@ static struct l2cap_chan *chan_create(void)
>> >> >>> if (!chan)
>> >> >>> return NULL;
>> >> >>>
>> >> >>> - l2cap_chan_set_defaults(chan);
>> >> >>> -
>> >> >>> - chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> >> >>> - chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> >> >>> - chan->imtu = 1280;
>> >> >>> + chan_init(chan);
>> >> >>>
>> >> >>> return chan;
>> >> >>> }
>> >> >>> @@ -745,19 +750,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan)
>> >> >>> ifup(dev->netdev);
>> >> >>> }
>> >> >>>
>> >> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
>> >> >>> +static inline int chan_new_conn_cb(struct l2cap_conn *conn,
>> >> >>> + struct l2cap_chan *pchan,
>> >> >>> + struct l2cap_chan *chan)
>> >> >>> {
>> >> >>> - struct l2cap_chan *chan;
>> >> >>> -
>> >> >>> - chan = chan_create();
>> >> >>> - if (!chan)
>> >> >>> - return NULL;
>> >> >>> -
>> >> >>> + chan_init(chan);
>>
>> ref1: chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; set
>> in chan_init.
>>
>> >> >>> chan->ops = pchan->ops;
>> >> >>>
>> >> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
>> >> >>> + __l2cap_chan_add(conn, chan);
>> >> >>> +
>> >> >>> BT_DBG("chan %p pchan %p", chan, pchan);
>> >> >>>
>> >> >>> - return chan;
>> >> >>> + return 0;
>> >> >>> }
>> >> >>>
>> >> >>> static void unregister_dev(struct lowpan_btle_dev *dev)
>> >> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> >> >>> index c4ccfbda9d78..62acf90837fb 100644
>> >> >>> --- a/net/bluetooth/l2cap_core.c
>> >> >>> +++ b/net/bluetooth/l2cap_core.c
>> >> >>> @@ -4007,6 +4007,31 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
>> >> >>> return 0;
>> >> >>> }
>> >> >>>
>> >> >>> +/* Allocate and initialise a channel for an incoming connection.
>> >> >>> + *
>> >> >>> + * ->new_connection() initialises the channel and links it into @conn with
>> >> >>> + * __l2cap_chan_add(). The l2cap_chan_create() reference becomes the one owned
>> >> >>> + * by the parent subsystem (l2cap_pi(sk)->chan, conn->smp or peer->chan) and is
>> >> >>> + * released by its teardown callback; the conn list reference is released by
>> >> >>> + * l2cap_chan_del().
>> >> >>> + */
>> >> >>> +static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
>> >> >>> + struct l2cap_chan *pchan)
>> >> >>> +{
>> >> >>> + struct l2cap_chan *chan;
>> >> >>> +
>> >> >>> + chan = l2cap_chan_create();
>> >> >>> + if (!chan)
>> >> >>> + return NULL;
>> >> >>> +
>> >> >>> + if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
>> >> >>> + l2cap_chan_put(chan);
>> >> >>> + return NULL;
>> >> >>> + }
>> >> >>
>> >> >> I don't quite get why we can't just place __l2cap_chan_add here
>> >> >> instead of having it called by new_connection callbacks?
>> >> >>
>> >> >
>> >> > It's specifically the l2cap_sock_new_connection_cb() case - the very
>> >> > use-after-free this patch fixes. The __l2cap_chan_add() has to happen while
>> >> > the parent lock is still held, and only the callback holds that lock.
>> >> >
>> >> > The reference counting on the new child chan starts at one ref,
>> >> > owned by the new socket:
>> >> >
>> >> > /* l2cap_new_connection() */
>> >> > chan = l2cap_chan_create(); /* refcount = 1 */
>> >> > if (!chan)
>> >> > return NULL;
>> >> >
>> >> > pchan->ops->new_connection(conn, pchan, chan);
>> >> >
>> >> > and inside the socket callback:
>> >> >
>> >> > /* l2cap_sock_new_connection_cb() */
>> >> > lock_sock(parent);
>> >> > ...
>> >> > sk = l2cap_sock_alloc(..., new_chan); /* sk owns the chan_create ref */
>> >> > ...
>> >> > l2cap_sock_init(sk, parent);
>> >> >
>> >> > __l2cap_chan_add(conn, new_chan); /* (A) conn list takes a ref */
>> >> > bt_accept_enqueue(parent, sk, false); /* (B) sk now on accept queue */
>> >> >
>> >> > release_sock(parent); /* (C) parent lock dropped */
>> >> > return 0;
>> >> >
>> >> > The moment we hit (C), sk is reachable through the parent's accept queue, so
>> >> > another task can grab and tear it down:
>> >> >
>> >> > accept() -> l2cap_sock_kill() -> l2cap_sock_put_chan()
>> >> > chan->data = NULL;
>> >> > l2cap_chan_put(chan); /* drops the sk's chan ref */
>> >> >
>> >> > If __l2cap_chan_add() at (A) hadn't already taken the conn list reference,
>> >> > that put would drop the last ref and free new_chan. Control then returns up
>> >> > to l2cap_new_connection(), which hands the now-freed chan back to
>> >> > l2cap_connect():
>> >> >
>> >> > /* l2cap_connect() - runs after the callback returns */
>> >> > chan = l2cap_new_connection(conn, pchan);
>> >> > if (!chan)
>> >> > goto response;
>> >> > ...
>> >> > bacpy(&chan->src, &conn->hcon->src); /* <-- UAF on freed chan */
>> >> > chan->psm = psm;
>> >> > chan->dcid = scid;
>> >> >
>> >> > The conn list reference taken at (A), before (C), is what keeps new_chan
>> >> > alive across the release_sock() window so l2cap_connect() can keep using it.
>> >> >
>> >> > So __l2cap_chan_add() can't move out to l2cap_new_connection(): by the time
>> >> > the callback returns, the parent lock is already dropped and chan may already
>> >> > be freed - which is exactly the race. It has to be taken inside the callback,
>> >> > under the parent lock, before the socket is exposed.
>> >
>> > If you do after l2cap_new_connection, yes, but what if you do before
>> > you call ->new_connect:
>> >
>> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> > index 3991c179b98f..cd864f400985 100644
>> > --- a/net/bluetooth/l2cap_core.c
>> > +++ b/net/bluetooth/l2cap_core.c
>> > @@ -4027,8 +4027,10 @@ static struct l2cap_chan
>> > *l2cap_new_connection(struct l2cap_conn *conn,
>> > if (!chan)
>> > return NULL;
>> >
>> > + __l2cap_chan_add(conn, chan);
>> > +
>> > if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
>> > - l2cap_chan_put(chan);
>> > + l2cap_chan_del(chan, 0);
>> > return NULL;
>> > }
>> >
>> > This means we have to call l2cap_chan_del if new_connection fails, but
>> > we can eliminate other functions having to call __l2cap_chan_add with
>> > this. If the issue is then having l2cap_chan_del called concurrently,
>> > we can add a l2cap_chan_hold; actually, we may need a reference anyway
>> > since sk can be released and call l2cap_chan_del, releasing both
>> > references.
>> >
>>
>> In:
>>
>> void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
>> {
>> ...
>> switch (chan->chan_type) {
>> case L2CAP_CHAN_CONN_ORIENTED:
>> ...
>> break;
>> case L2CAP_CHAN_CONN_LESS:
>> ...
>> break;
>> case L2CAP_CHAN_FIXED:
>> ...
>> break;
>> default:
>> ...
>> }
>> }
>>
>> Apparently it depends on chan->chan_type and it is set by ref1.
>> It is only set in cb function.
>
> Ok, this probably needs fixing in 6lowpan. These defaults should come
> from the parent channel, similar to how it's done in l2cap_sock_init.
> Perhaps we should promote that logic to l2cap_chan_set_defaults
> passing pchan so it can initialize the channel before calling
> __l2cap_chan_add.
I can fix that in a separate patch.
Could you please merge it first?
Best,
Siwei
^ permalink raw reply
* Re: [PATCH v9 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()
From: Luiz Augusto von Dentz @ 2026-06-10 16:59 UTC (permalink / raw)
To: Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <3cf8c12c-10c3-4ba8-9037-7e41a820d23d@app.fastmail.com>
Hi Siwei,
On Wed, Jun 10, 2026 at 12:33 PM Siwei Zhang <oss@fourdim.xyz> wrote:
>
>
>
> On Wed, Jun 10, 2026, at 11:57 AM, Luiz Augusto von Dentz wrote:
> > Hi Siwei,
> >
> > On Mon, Jun 8, 2026 at 9:29 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >>
> >> Hi Luiz,
> >>
> >> On Thu, Jun 4, 2026, at 11:52 AM, Siwei Zhang wrote:
> >> > Hi Luiz,
> >> >
> >> > On Wed, Jun 3, 2026, at 2:17 PM, Luiz Augusto von Dentz wrote:
> >> >> Hi Siwei,
> >> >>
> >> >> On Wed, Jun 3, 2026 at 11:09 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >> >>>
> >> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
> >> >>> release_sock(parent). Once the parent lock is released, the child
> >> >>> socket sk can be freed by another task.
> >> >>>
> >> >>> Allocate the channel outside the func to prevent this.
> >> >>>
> >> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
> >> >>> Cc: stable@kernel.org
> >> >>> Assisted-by: Claude:claude-opus-4-8
> >> >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> >> >>> ---
> >> >>> include/net/bluetooth/l2cap.h | 10 +++--
> >> >>> net/bluetooth/6lowpan.c | 31 +++++++------
> >> >>> net/bluetooth/l2cap_core.c | 41 ++++++++++++-----
> >> >>> net/bluetooth/l2cap_sock.c | 83 +++++++++++++++++++++--------------
> >> >>> net/bluetooth/smp.c | 17 ++++---
> >> >>> 5 files changed, 113 insertions(+), 69 deletions(-)
> >> >>>
> >> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >> >>> index e0a1f2293679..7f5e4647f6e0 100644
> >> >>> --- a/include/net/bluetooth/l2cap.h
> >> >>> +++ b/include/net/bluetooth/l2cap.h
> >> >>> @@ -620,7 +620,9 @@ struct l2cap_chan {
> >> >>> struct l2cap_ops {
> >> >>> char *name;
> >> >>>
> >> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
> >> >>> + int (*new_connection)(struct l2cap_conn *conn,
> >> >>> + struct l2cap_chan *chan,
> >> >>> + struct l2cap_chan *new_chan);
> >> >>> int (*recv) (struct l2cap_chan * chan,
> >> >>> struct sk_buff *skb);
> >> >>> void (*teardown) (struct l2cap_chan *chan, int err);
> >> >>> @@ -884,9 +886,11 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
> >> >>> return (seq + 1) % (chan->tx_win_max + 1);
> >> >>> }
> >> >>>
> >> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> >> >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_conn *conn,
> >> >>> + struct l2cap_chan *chan,
> >> >>> + struct l2cap_chan *new_chan)
> >> >>> {
> >> >>> - return NULL;
> >> >>> + return -EOPNOTSUPP;
> >> >>> }
> >> >>>
> >> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb)
> >> >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> >> >>> index cb1e329d66fd..94863af97a44 100644
> >> >>> --- a/net/bluetooth/6lowpan.c
> >> >>> +++ b/net/bluetooth/6lowpan.c
> >> >>> @@ -624,6 +624,15 @@ static bool is_bt_6lowpan(struct hci_conn *hcon)
> >> >>> return true;
> >> >>> }
> >> >>>
> >> >>> +static void chan_init(struct l2cap_chan *chan)
> >> >>> +{
> >> >>> + l2cap_chan_set_defaults(chan);
> >> >>> +
> >> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> >> >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >> >>> + chan->imtu = 1280;
> >> >>> +}
> >> >>> +
> >> >>> static struct l2cap_chan *chan_create(void)
> >> >>> {
> >> >>> struct l2cap_chan *chan;
> >> >>> @@ -632,11 +641,7 @@ static struct l2cap_chan *chan_create(void)
> >> >>> if (!chan)
> >> >>> return NULL;
> >> >>>
> >> >>> - l2cap_chan_set_defaults(chan);
> >> >>> -
> >> >>> - chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> >> >>> - chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >> >>> - chan->imtu = 1280;
> >> >>> + chan_init(chan);
> >> >>>
> >> >>> return chan;
> >> >>> }
> >> >>> @@ -745,19 +750,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan)
> >> >>> ifup(dev->netdev);
> >> >>> }
> >> >>>
> >> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
> >> >>> +static inline int chan_new_conn_cb(struct l2cap_conn *conn,
> >> >>> + struct l2cap_chan *pchan,
> >> >>> + struct l2cap_chan *chan)
> >> >>> {
> >> >>> - struct l2cap_chan *chan;
> >> >>> -
> >> >>> - chan = chan_create();
> >> >>> - if (!chan)
> >> >>> - return NULL;
> >> >>> -
> >> >>> + chan_init(chan);
>
> ref1: chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; set
> in chan_init.
>
> >> >>> chan->ops = pchan->ops;
> >> >>>
> >> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
> >> >>> + __l2cap_chan_add(conn, chan);
> >> >>> +
> >> >>> BT_DBG("chan %p pchan %p", chan, pchan);
> >> >>>
> >> >>> - return chan;
> >> >>> + return 0;
> >> >>> }
> >> >>>
> >> >>> static void unregister_dev(struct lowpan_btle_dev *dev)
> >> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> >>> index c4ccfbda9d78..62acf90837fb 100644
> >> >>> --- a/net/bluetooth/l2cap_core.c
> >> >>> +++ b/net/bluetooth/l2cap_core.c
> >> >>> @@ -4007,6 +4007,31 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
> >> >>> return 0;
> >> >>> }
> >> >>>
> >> >>> +/* Allocate and initialise a channel for an incoming connection.
> >> >>> + *
> >> >>> + * ->new_connection() initialises the channel and links it into @conn with
> >> >>> + * __l2cap_chan_add(). The l2cap_chan_create() reference becomes the one owned
> >> >>> + * by the parent subsystem (l2cap_pi(sk)->chan, conn->smp or peer->chan) and is
> >> >>> + * released by its teardown callback; the conn list reference is released by
> >> >>> + * l2cap_chan_del().
> >> >>> + */
> >> >>> +static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
> >> >>> + struct l2cap_chan *pchan)
> >> >>> +{
> >> >>> + struct l2cap_chan *chan;
> >> >>> +
> >> >>> + chan = l2cap_chan_create();
> >> >>> + if (!chan)
> >> >>> + return NULL;
> >> >>> +
> >> >>> + if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
> >> >>> + l2cap_chan_put(chan);
> >> >>> + return NULL;
> >> >>> + }
> >> >>
> >> >> I don't quite get why we can't just place __l2cap_chan_add here
> >> >> instead of having it called by new_connection callbacks?
> >> >>
> >> >
> >> > It's specifically the l2cap_sock_new_connection_cb() case - the very
> >> > use-after-free this patch fixes. The __l2cap_chan_add() has to happen while
> >> > the parent lock is still held, and only the callback holds that lock.
> >> >
> >> > The reference counting on the new child chan starts at one ref,
> >> > owned by the new socket:
> >> >
> >> > /* l2cap_new_connection() */
> >> > chan = l2cap_chan_create(); /* refcount = 1 */
> >> > if (!chan)
> >> > return NULL;
> >> >
> >> > pchan->ops->new_connection(conn, pchan, chan);
> >> >
> >> > and inside the socket callback:
> >> >
> >> > /* l2cap_sock_new_connection_cb() */
> >> > lock_sock(parent);
> >> > ...
> >> > sk = l2cap_sock_alloc(..., new_chan); /* sk owns the chan_create ref */
> >> > ...
> >> > l2cap_sock_init(sk, parent);
> >> >
> >> > __l2cap_chan_add(conn, new_chan); /* (A) conn list takes a ref */
> >> > bt_accept_enqueue(parent, sk, false); /* (B) sk now on accept queue */
> >> >
> >> > release_sock(parent); /* (C) parent lock dropped */
> >> > return 0;
> >> >
> >> > The moment we hit (C), sk is reachable through the parent's accept queue, so
> >> > another task can grab and tear it down:
> >> >
> >> > accept() -> l2cap_sock_kill() -> l2cap_sock_put_chan()
> >> > chan->data = NULL;
> >> > l2cap_chan_put(chan); /* drops the sk's chan ref */
> >> >
> >> > If __l2cap_chan_add() at (A) hadn't already taken the conn list reference,
> >> > that put would drop the last ref and free new_chan. Control then returns up
> >> > to l2cap_new_connection(), which hands the now-freed chan back to
> >> > l2cap_connect():
> >> >
> >> > /* l2cap_connect() - runs after the callback returns */
> >> > chan = l2cap_new_connection(conn, pchan);
> >> > if (!chan)
> >> > goto response;
> >> > ...
> >> > bacpy(&chan->src, &conn->hcon->src); /* <-- UAF on freed chan */
> >> > chan->psm = psm;
> >> > chan->dcid = scid;
> >> >
> >> > The conn list reference taken at (A), before (C), is what keeps new_chan
> >> > alive across the release_sock() window so l2cap_connect() can keep using it.
> >> >
> >> > So __l2cap_chan_add() can't move out to l2cap_new_connection(): by the time
> >> > the callback returns, the parent lock is already dropped and chan may already
> >> > be freed - which is exactly the race. It has to be taken inside the callback,
> >> > under the parent lock, before the socket is exposed.
> >
> > If you do after l2cap_new_connection, yes, but what if you do before
> > you call ->new_connect:
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 3991c179b98f..cd864f400985 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4027,8 +4027,10 @@ static struct l2cap_chan
> > *l2cap_new_connection(struct l2cap_conn *conn,
> > if (!chan)
> > return NULL;
> >
> > + __l2cap_chan_add(conn, chan);
> > +
> > if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
> > - l2cap_chan_put(chan);
> > + l2cap_chan_del(chan, 0);
> > return NULL;
> > }
> >
> > This means we have to call l2cap_chan_del if new_connection fails, but
> > we can eliminate other functions having to call __l2cap_chan_add with
> > this. If the issue is then having l2cap_chan_del called concurrently,
> > we can add a l2cap_chan_hold; actually, we may need a reference anyway
> > since sk can be released and call l2cap_chan_del, releasing both
> > references.
> >
>
> In:
>
> void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> {
> ...
> switch (chan->chan_type) {
> case L2CAP_CHAN_CONN_ORIENTED:
> ...
> break;
> case L2CAP_CHAN_CONN_LESS:
> ...
> break;
> case L2CAP_CHAN_FIXED:
> ...
> break;
> default:
> ...
> }
> }
>
> Apparently it depends on chan->chan_type and it is set by ref1.
> It is only set in cb function.
Ok, this probably needs fixing in 6lowpan. These defaults should come
from the parent channel, similar to how it's done in l2cap_sock_init.
Perhaps we should promote that logic to l2cap_chan_set_defaults
passing pchan so it can initialize the channel before calling
__l2cap_chan_add.
^ permalink raw reply
* Re: [PATCH] Bluetooth: hci_bcm4377: Use named initializers for pci_device_id array
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-10 16:59 UTC (permalink / raw)
To: Sven Peter, Janne Grunau, Neal Gompa, Marcel Holtmann,
Luiz Augusto von Dentz
Cc: Markus Schneider-Pargmann, asahi, linux-arm-kernel,
linux-bluetooth, linux-kernel
In-Reply-To: <20260504160940.2168650-2-u.kleine-koenig@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]
On Mon, May 04, 2026 at 06:09:40PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Initializing a struct using list initializers is hard to read, compared
> to that using named initializers is more ideomatic. Convert the macro
> used to assign values in the driver's pci_device_id array accordingly.
>
> This change doesn't introduce any changes to the compiled array on an
> x86 and an arm64 build.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> Hello,
>
> this is a preparing change for making struct pci_device_id::driver_data an
> anonymous union (similar to
> https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/).
> This requires named initializers for .driver_data. But even without that
> this is a nice cleanup making the macro better readable.
>
> Gcc is happy with simplifying the assignment further using
> PCI_VDEVICE(BROADCOM, BCM ## id ## _DEVICE_ID), but this is a bit fishy
> because PCI_VDEVICE also assigns .class and .class_mask (using list
> initializers), so I didn't convert that.
In the meantime I learned that doing that would break W=1 builds, so it
was a good choice to not go that path.
> Once all pci_device_id use
> named initializers, the two zeros can be dropped from PCI_VDEVICE and
> this entry simplified accordingly.
Is this patch still on someone's radar? Ideally for application in time
for 7.2-rc1?
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [bluez/bluez]
From: BluezTestBot @ 2026-06-10 16:50 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/heads/1092801
Home: https://github.com/bluez/bluez
To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
^ permalink raw reply
* Re: [PATCH v6 6/9] dt-bindings: connector: m2: Add M.2 1620 LGA soldered down connector
From: Manivannan Sadhasivam @ 2026-06-10 16:44 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Mark Pearson, Dmitry Baryshkov, Rob Herring,
Manivannan Sadhasivam, Greg KH, Jiri Slaby, Nathan Chancellor,
Nicolas Schier, Hans de Goede, Ilpo Järvinen,
Derek J . Clark, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86@vger.kernel.org, linux-pci,
devicetree, linux-arm-msm, linux-bluetooth, linux-pm,
linux-acpi@vger.kernel.org
In-Reply-To: <acv2f1qbqu4PlSL1@linaro.org>
On Tue, Mar 31, 2026 at 06:29:51PM +0200, Stephan Gerhold wrote:
> On Wed, Mar 25, 2026 at 05:36:08PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Mar 23, 2026 at 01:23:07PM -0400, Mark Pearson wrote:
> > > On Mon, Mar 23, 2026, at 12:52 PM, Manivannan Sadhasivam wrote:
> > > > On Mon, Mar 23, 2026 at 06:45:15PM +0200, Dmitry Baryshkov wrote:
> > > >> On Mon, Mar 23, 2026 at 09:26:04PM +0530, Manivannan Sadhasivam wrote:
> > > >> > On Mon, Mar 23, 2026 at 05:14:30PM +0200, Dmitry Baryshkov wrote:
> > > >> > > On Mon, Mar 23, 2026 at 07:14:25PM +0530, Manivannan Sadhasivam wrote:
> > > >> > > > On Mon, Mar 23, 2026 at 08:39:55AM -0500, Rob Herring wrote:
> > > >> > > > > On Mon, Mar 23, 2026 at 7:16 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > >> > > > > >
> > > >> > > > > > On Sun, Mar 22, 2026 at 06:37:13PM -0500, Rob Herring wrote:
> > > >> > > > > > > On Tue, Mar 17, 2026 at 09:59:56AM +0530, Manivannan Sadhasivam wrote:
> > > >> > > > > > > > Lenovo Thinkpad T14s is found to have a soldered down version of M.2 1620
> > > >> > > > > > > > LGA connector. Though, there is no 1620 LGA form factor defined in the M.2
> > > >> > > > > > > > spec, it looks very similar to the M.2 Key E connector. So add the
> > > >> > > > > > > > "pcie-m2-1620-lga-connector" compatible with "pcie-m2-e-connector" fallback
> > > >> > > > > > > > to reuse the Key E binding.
> > > >> > > > > > >
> > > >> > > > > > > What is LGA?
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > > > Land Grid Array
> > > >> > > > > >
> > > >> > > > > > > If not in the spec, is it really something generic?
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > > > Good question. Yes and No! LGA is not something that Lenovo only uses. Other
> > > >> > > > > > vendors may also use this form factor. PCIe connectors are full of innovation as
> > > >> > > > > > the spec gives room for hardware designers to be as innovative as possible to
> > > >> > > > > > save the BOM cost.
> > > >> > > > >
> > > >> > > > > innovation == incompatible changes
> > > >> > > > >
> > > >> > > >
> > > >> > > > Yes, I was trying to sound nice :)
> > > >> > > >
> > > >> > > > > > This is why I do not want to make it Lenovo specific. But if you prefer that, I
> > > >> > > > > > can name it as "lenovo,pcie-m2-1620-lga-connector".
> > > >> > > > >
> > > >> > > > > Depends if you think that s/w needs to know the differences. Hard to
> > > >> > > > > say with a sample size of 1.
> > > >> > > > >
> > > >> > > >
> > > >> > > > Sure. Will add the 'lenovo' prefix then.
> > > >> > >
> > > >> > > Is it really Lenovo? Or is it some other module vendor, whose LGAs are
> > > >> > > being used by Lenovo?
> > > >> > >
> > > >> > > I remember that DB820c also used some kind of a module for the WiFi card
> > > >> > > (which might be M.2 compatible or might not, I can't find exact docs at
> > > >> > > this point).
> > > >> > >
> > > >> >
> > > >> > I don't know. These kind of designs might be reused by several vendors. But
> > > >> > considering that we should not make it generic, I'd go with Lenovo as that's
> > > >> > the only vendor we know as of now.
> > > >>
> > > >> ... and later we learn that other vendors use the same idea /pinout,
> > > >> then nothing stops us from still telling that it's a
> > > >> "lenovo,pcie-m2-something-lga".
> > > >>
> > > >
> > > > How do you possibly know whether a single vendor has introduced this form factor
> > > > or reused by multiple ones? Atleast, I don't have access to such a source to
> > > > confirm.
> > > >
> > > I've not really been following this thread/patchset in detail; but want me to try and check with the T14s platform team if this device is specifically made for us (Lenovo) or not?
> > > I doubt it is - we just don't do that usually, but I can go and ask the question if it will help resolve this (with the caveat that it could hold up the review for a bit and I may not be able to get a straight answer)
> > >
> >
> > I can drop this specific patch in the meantime.
> >
> > > My vote (for what little it's worth) would be to make it non-Lenovo specific. Then when the same part causes issues on another vendors platform I won't get asked questions about why Lenovo is breaking <other vendor> :)
> > >
> >
> > Even if Lenovo prefix is used, it won't break other vendors. Just that we will
> > end up adding more compatibles.
> >
> > Anyhow, I'll wait for your reply and drop this patch for next revision.
> >
>
> If you need a vendor prefix, I think "qcom," would be more appropriate
> than Lenovo. This form factor is used by most vendors for recent
> soldered Qualcomm-based wireless cards, not just Lenovo:
>
> - Dell XPS 13 9345 has exactly the same soldered M.2 card, I assume
> there are several other vendors as well.
>
> - https://www.sparklan.com/product/wnsq-290be/ is a third-party
> (Qualcomm-based) M.2 LGA 1620 card, in the block diagram the
> pinout is called "QM.2 1620 LGA 168pin".
>
> - If you press F9 while booting the ThinkPad T14s, you should get to a
> screen with "Regulatory Information". For the T14s, this screen says
> "Contains FCC ID: J9C-QCNCM825". This is the WiFi/BT module in the
> soldered form factor. If you look that up on the FCC website, the
> applicant for this module is "Qualcomm Technologies, Inc.". This
> seems to be some kind of "modular certification" that vendors can
> reuse/adapt without going through the whole process again.
>
> Perhaps you should ask around inside Qualcomm? :-)
>
Sorry for getting back after this long. I did ask around, but our HW folks are
saying that Qcom is not the first one to use LGA M.2 modules. They claim that
other vendors also do that.
But for this specific card, it should be fine to use the 'qcom' prefix as
apparently the module was supplied by Qcom.
I'll submit the bindings patch together with DTS change for T14s.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Luiz Augusto von Dentz @ 2026-06-10 16:43 UTC (permalink / raw)
To: Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <20260610150704.1234558-1-oss@fourdim.xyz>
Hi Siwei,
On Wed, Jun 10, 2026 at 11:07 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>
> hci_abort_conn() read hci_skb_event(hdev->sent_cmd) when a connection
> was pending, but hdev->sent_cmd can be NULL while req_status is still
> HCI_REQ_PEND, leading to a NULL pointer dereference and a general
> protection fault from the hci_rx_work() receive path.
>
> Instead of inspecting hdev->sent_cmd, track the in-flight create
> connection command with a new per-connection HCI_CONN_CREATE_CONN flag
> and route all cancellation through hci_cancel_connect_sync(), which
> dequeues the command if still queued, or cancels the pending request if
> this connection owns the in-flight create command. CIS uses the same
> path via the existing HCI_CONN_CREATE_CIS flag.
>
> Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 21 ++----------
> net/bluetooth/hci_sync.c | 58 +++++++++++++++++++++++++-------
> 3 files changed, 50 insertions(+), 30 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index aa600fbf9a53..c90ca6173680 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -988,6 +988,7 @@ enum {
> HCI_CONN_AUTH_FAILURE,
> HCI_CONN_PER_ADV,
> HCI_CONN_BIG_CREATED,
> + HCI_CONN_CREATE_CONN,
Use just HCI_CONN_CREATE
> HCI_CONN_CREATE_CIS,
> HCI_CONN_CREATE_BIG_SYNC,
> HCI_CONN_BIG_SYNC,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 54eabaa46960..eba4a548bef5 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -3181,26 +3181,11 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
>
> conn->abort_reason = reason;
>
> - /* If the connection is pending check the command opcode since that
> - * might be blocking on hci_cmd_sync_work while waiting its respective
> - * event so we need to hci_cmd_sync_cancel to cancel it.
> - *
> - * hci_connect_le serializes the connection attempts so only one
> - * connection can be in BT_CONNECT at time.
> + /* Cancel the connect attempt. A return of 0 means the create command
> + * was still queued and got dequeued, so there is nothing to disconnect.
> */
> - if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
> - switch (hci_skb_event(hdev->sent_cmd)) {
> - case HCI_EV_CONN_COMPLETE:
> - case HCI_EV_LE_CONN_COMPLETE:
> - case HCI_EV_LE_ENHANCED_CONN_COMPLETE:
> - case HCI_EVT_LE_CIS_ESTABLISHED:
> - hci_cmd_sync_cancel(hdev, ECANCELED);
> - break;
> - }
> - /* Cancel connect attempt if still queued/pending */
> - } else if (!hci_cancel_connect_sync(hdev, conn)) {
> + if (!hci_cancel_connect_sync(hdev, conn))
> return 0;
> - }
>
> /* Run immediately if on cmd_sync_work since this may be called
> * as a result to MGMT_OP_DISCONNECT/MGMT_OP_UNPAIR which does
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index df23245d6ccd..08917b8167de 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -6668,6 +6668,12 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
> &own_addr_type);
> if (err)
> goto done;
> +
> + /* Mark create connection in flight so hci_cancel_connect_sync() can
> + * cancel it while blocking on the connection complete event.
> + */
> + set_bit(HCI_CONN_CREATE_CONN, &conn->flags);
> +
> /* Send command LE Extended Create Connection if supported */
> if (use_ext_conn(hdev)) {
> err = hci_le_ext_create_conn_sync(hdev, conn, own_addr_type);
> @@ -6703,6 +6709,8 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
> conn->conn_timeout, NULL);
>
> done:
> + clear_bit(HCI_CONN_CREATE_CONN, &conn->flags);
> +
> if (err == -ETIMEDOUT)
> hci_le_connect_cancel_sync(hdev, conn, 0x00);
>
> @@ -6982,10 +6990,19 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
> else
> cp.role_switch = 0x00;
>
> - return __hci_cmd_sync_status_sk(hdev, HCI_OP_CREATE_CONN,
> - sizeof(cp), &cp,
> - HCI_EV_CONN_COMPLETE,
> - conn->conn_timeout, NULL);
> + /* Mark create connection in flight so hci_cancel_connect_sync() can
> + * cancel it while blocking on the connection complete event.
> + */
> + set_bit(HCI_CONN_CREATE_CONN, &conn->flags);
> +
> + err = __hci_cmd_sync_status_sk(hdev, HCI_OP_CREATE_CONN,
> + sizeof(cp), &cp,
> + HCI_EV_CONN_COMPLETE,
> + conn->conn_timeout, NULL);
> +
> + clear_bit(HCI_CONN_CREATE_CONN, &conn->flags);
> +
> + return err;
> }
>
> int hci_connect_acl_sync(struct hci_dev *hdev, struct hci_conn *conn)
> @@ -7039,20 +7056,37 @@ int hci_connect_le_sync(struct hci_dev *hdev, struct hci_conn *conn)
>
> int hci_cancel_connect_sync(struct hci_dev *hdev, struct hci_conn *conn)
> {
> - if (conn->state != BT_OPEN)
> - return -EINVAL;
> + hci_cmd_sync_work_func_t func = NULL;
> + hci_cmd_sync_work_destroy_t destroy = NULL;
> + int create_flag = -1;
>
> switch (conn->type) {
> case ACL_LINK:
> - return !hci_cmd_sync_dequeue_once(hdev,
> - hci_acl_create_conn_sync,
> - conn, NULL);
> + func = hci_acl_create_conn_sync;
> + create_flag = HCI_CONN_CREATE_CONN;
> + break;
> case LE_LINK:
> - return !hci_cmd_sync_dequeue_once(hdev, hci_le_create_conn_sync,
> - conn, create_le_conn_complete);
> + func = hci_le_create_conn_sync;
> + destroy = create_le_conn_complete;
> + create_flag = HCI_CONN_CREATE_CONN;
> + break;
> + case CIS_LINK:
> + /* LE Create CIS is shared by the whole CIG and cannot be
> + * dequeued per-connection; only cancel it in-flight below.
> + */
> + create_flag = HCI_CONN_CREATE_CIS;
> + break;
> + default:
> + return -ENOENT;
> }
>
> - return -ENOENT;
> + if (func && hci_cmd_sync_dequeue_once(hdev, func, conn, destroy))
> + return 0;
> +
> + if (create_flag >= 0 && test_bit(create_flag, &conn->flags))
> + hci_cmd_sync_cancel(hdev, ECANCELED);
Hmm, I'm not sure I like how this is done here, it seems to always
attempt to dequeue even if the flag indicates it is pending. If that
is intentional, you should add a comment.
> + return -EBUSY;
> }
>
> int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
> --
> 2.54.0
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: qcom: monaco-arduino-monza: Add QCA2066 M.2 WiFi/BT support
From: Manivannan Sadhasivam @ 2026-06-10 16:42 UTC (permalink / raw)
To: Loic Poulain
Cc: Dmitry Baryshkov, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pci,
linux-pm, linux-kernel, linux-arm-msm, linux-bluetooth,
devicetree
In-Reply-To: <CAFEp6-3Gbd1gzfeu5xdfBJixL6JXaoSFkRUsBjOji0ZEOHHyvw@mail.gmail.com>
On Wed, May 20, 2026 at 04:41:18PM +0200, Loic Poulain wrote:
> On Wed, May 20, 2026 at 4:36 PM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >
> > On Wed, May 20, 2026 at 04:29:40PM +0200, Loic Poulain wrote:
> > > On Wed, May 20, 2026 at 2:34 PM Dmitry Baryshkov
> > > <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > > >
> > > > On Wed, May 20, 2026 at 01:01:44PM +0200, Loic Poulain wrote:
> > > > > Add support for the QCA2066 (QCNFA765) WiFi/Bluetooth module on the
> > > > > Arduino VENTUNO Q board. The module is interfaced via LGA and is
> > > > > compatible with the M.2 Key E.
> > > > >
> > > > > Add wireless-lga-connector node using pcie-m2-e-connector binding,
> > > > > connecting PCIe port 0 to the WiFi interface and UART10 port 3 to
> > > > > the Bluetooth interface.
> > > > >
> > > > > Add pcie@1,0 downstream port node with pciclass,0604 compatible so
> > > > > the pci-pwrctrl driver can acquire the power sequencer and enable
> > > > > the M.2 slot before PCIe enumeration.
> > > > >
> > > > > Add nfa725b_default_state pinctrl for the W_DISABLE1/2 GPIOs
> > > > > (gpio56/gpio55) used by the power sequencer.
> > > > >
> > > > > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > > > > ---
> > > > > arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts | 65 +++++++++++++++++++++++
> > > > > 1 file changed, 65 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> > > > > index 93ed575817af1c5e903662c209ead629fe202ee2..6fcad77f320cb82eccb6f07244d185abfb1976d9 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> > > > > @@ -154,6 +154,39 @@ vreg_nvme: regulator-3p3-m2 {
> > > > > enable-active-high;
> > > > > startup-delay-us = <20000>;
> > > > > };
> > > > > +
> > > > > + wireless-lga-connector {
> > > > > + compatible = "pcie-m2-e-connector";
> > > >
> > > > I think it was discussed that LGA can't be an actual M.2 E-key
> > > > connector.
> > >
> > > I am not sure I followed this discussion. Do you mean that I should
> > > introduce a dedicated LGA/vendor-compatible string in the compatible
> > > list of the pcie-m2-e-connector binding, or that LGA-based designs
> > > should not be described using the pcie-m2-e-connector binding (graph
> > > representation)?
> >
> > I think, it should be a separate, vendor-specific compat (maybe using
> > m2-e as a fallback).
>
You can use "qcom,pcie-m2-<size>-lga-connector", where size depends on the
module size, like 1620.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v9 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()
From: Siwei Zhang @ 2026-06-10 16:32 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <CABBYNZK2tzGe9CKcwBb7SxGzE94Y04z_VvaoFLoQPJ=3m=MUgQ@mail.gmail.com>
On Wed, Jun 10, 2026, at 11:57 AM, Luiz Augusto von Dentz wrote:
> Hi Siwei,
>
> On Mon, Jun 8, 2026 at 9:29 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>>
>> Hi Luiz,
>>
>> On Thu, Jun 4, 2026, at 11:52 AM, Siwei Zhang wrote:
>> > Hi Luiz,
>> >
>> > On Wed, Jun 3, 2026, at 2:17 PM, Luiz Augusto von Dentz wrote:
>> >> Hi Siwei,
>> >>
>> >> On Wed, Jun 3, 2026 at 11:09 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >>>
>> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
>> >>> release_sock(parent). Once the parent lock is released, the child
>> >>> socket sk can be freed by another task.
>> >>>
>> >>> Allocate the channel outside the func to prevent this.
>> >>>
>> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
>> >>> Cc: stable@kernel.org
>> >>> Assisted-by: Claude:claude-opus-4-8
>> >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
>> >>> ---
>> >>> include/net/bluetooth/l2cap.h | 10 +++--
>> >>> net/bluetooth/6lowpan.c | 31 +++++++------
>> >>> net/bluetooth/l2cap_core.c | 41 ++++++++++++-----
>> >>> net/bluetooth/l2cap_sock.c | 83 +++++++++++++++++++++--------------
>> >>> net/bluetooth/smp.c | 17 ++++---
>> >>> 5 files changed, 113 insertions(+), 69 deletions(-)
>> >>>
>> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> >>> index e0a1f2293679..7f5e4647f6e0 100644
>> >>> --- a/include/net/bluetooth/l2cap.h
>> >>> +++ b/include/net/bluetooth/l2cap.h
>> >>> @@ -620,7 +620,9 @@ struct l2cap_chan {
>> >>> struct l2cap_ops {
>> >>> char *name;
>> >>>
>> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
>> >>> + int (*new_connection)(struct l2cap_conn *conn,
>> >>> + struct l2cap_chan *chan,
>> >>> + struct l2cap_chan *new_chan);
>> >>> int (*recv) (struct l2cap_chan * chan,
>> >>> struct sk_buff *skb);
>> >>> void (*teardown) (struct l2cap_chan *chan, int err);
>> >>> @@ -884,9 +886,11 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
>> >>> return (seq + 1) % (chan->tx_win_max + 1);
>> >>> }
>> >>>
>> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
>> >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_conn *conn,
>> >>> + struct l2cap_chan *chan,
>> >>> + struct l2cap_chan *new_chan)
>> >>> {
>> >>> - return NULL;
>> >>> + return -EOPNOTSUPP;
>> >>> }
>> >>>
>> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb)
>> >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> >>> index cb1e329d66fd..94863af97a44 100644
>> >>> --- a/net/bluetooth/6lowpan.c
>> >>> +++ b/net/bluetooth/6lowpan.c
>> >>> @@ -624,6 +624,15 @@ static bool is_bt_6lowpan(struct hci_conn *hcon)
>> >>> return true;
>> >>> }
>> >>>
>> >>> +static void chan_init(struct l2cap_chan *chan)
>> >>> +{
>> >>> + l2cap_chan_set_defaults(chan);
>> >>> +
>> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> >>> + chan->imtu = 1280;
>> >>> +}
>> >>> +
>> >>> static struct l2cap_chan *chan_create(void)
>> >>> {
>> >>> struct l2cap_chan *chan;
>> >>> @@ -632,11 +641,7 @@ static struct l2cap_chan *chan_create(void)
>> >>> if (!chan)
>> >>> return NULL;
>> >>>
>> >>> - l2cap_chan_set_defaults(chan);
>> >>> -
>> >>> - chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> >>> - chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> >>> - chan->imtu = 1280;
>> >>> + chan_init(chan);
>> >>>
>> >>> return chan;
>> >>> }
>> >>> @@ -745,19 +750,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan)
>> >>> ifup(dev->netdev);
>> >>> }
>> >>>
>> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
>> >>> +static inline int chan_new_conn_cb(struct l2cap_conn *conn,
>> >>> + struct l2cap_chan *pchan,
>> >>> + struct l2cap_chan *chan)
>> >>> {
>> >>> - struct l2cap_chan *chan;
>> >>> -
>> >>> - chan = chan_create();
>> >>> - if (!chan)
>> >>> - return NULL;
>> >>> -
>> >>> + chan_init(chan);
ref1: chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; set
in chan_init.
>> >>> chan->ops = pchan->ops;
>> >>>
>> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
>> >>> + __l2cap_chan_add(conn, chan);
>> >>> +
>> >>> BT_DBG("chan %p pchan %p", chan, pchan);
>> >>>
>> >>> - return chan;
>> >>> + return 0;
>> >>> }
>> >>>
>> >>> static void unregister_dev(struct lowpan_btle_dev *dev)
>> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> >>> index c4ccfbda9d78..62acf90837fb 100644
>> >>> --- a/net/bluetooth/l2cap_core.c
>> >>> +++ b/net/bluetooth/l2cap_core.c
>> >>> @@ -4007,6 +4007,31 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
>> >>> return 0;
>> >>> }
>> >>>
>> >>> +/* Allocate and initialise a channel for an incoming connection.
>> >>> + *
>> >>> + * ->new_connection() initialises the channel and links it into @conn with
>> >>> + * __l2cap_chan_add(). The l2cap_chan_create() reference becomes the one owned
>> >>> + * by the parent subsystem (l2cap_pi(sk)->chan, conn->smp or peer->chan) and is
>> >>> + * released by its teardown callback; the conn list reference is released by
>> >>> + * l2cap_chan_del().
>> >>> + */
>> >>> +static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
>> >>> + struct l2cap_chan *pchan)
>> >>> +{
>> >>> + struct l2cap_chan *chan;
>> >>> +
>> >>> + chan = l2cap_chan_create();
>> >>> + if (!chan)
>> >>> + return NULL;
>> >>> +
>> >>> + if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
>> >>> + l2cap_chan_put(chan);
>> >>> + return NULL;
>> >>> + }
>> >>
>> >> I don't quite get why we can't just place __l2cap_chan_add here
>> >> instead of having it called by new_connection callbacks?
>> >>
>> >
>> > It's specifically the l2cap_sock_new_connection_cb() case - the very
>> > use-after-free this patch fixes. The __l2cap_chan_add() has to happen while
>> > the parent lock is still held, and only the callback holds that lock.
>> >
>> > The reference counting on the new child chan starts at one ref,
>> > owned by the new socket:
>> >
>> > /* l2cap_new_connection() */
>> > chan = l2cap_chan_create(); /* refcount = 1 */
>> > if (!chan)
>> > return NULL;
>> >
>> > pchan->ops->new_connection(conn, pchan, chan);
>> >
>> > and inside the socket callback:
>> >
>> > /* l2cap_sock_new_connection_cb() */
>> > lock_sock(parent);
>> > ...
>> > sk = l2cap_sock_alloc(..., new_chan); /* sk owns the chan_create ref */
>> > ...
>> > l2cap_sock_init(sk, parent);
>> >
>> > __l2cap_chan_add(conn, new_chan); /* (A) conn list takes a ref */
>> > bt_accept_enqueue(parent, sk, false); /* (B) sk now on accept queue */
>> >
>> > release_sock(parent); /* (C) parent lock dropped */
>> > return 0;
>> >
>> > The moment we hit (C), sk is reachable through the parent's accept queue, so
>> > another task can grab and tear it down:
>> >
>> > accept() -> l2cap_sock_kill() -> l2cap_sock_put_chan()
>> > chan->data = NULL;
>> > l2cap_chan_put(chan); /* drops the sk's chan ref */
>> >
>> > If __l2cap_chan_add() at (A) hadn't already taken the conn list reference,
>> > that put would drop the last ref and free new_chan. Control then returns up
>> > to l2cap_new_connection(), which hands the now-freed chan back to
>> > l2cap_connect():
>> >
>> > /* l2cap_connect() - runs after the callback returns */
>> > chan = l2cap_new_connection(conn, pchan);
>> > if (!chan)
>> > goto response;
>> > ...
>> > bacpy(&chan->src, &conn->hcon->src); /* <-- UAF on freed chan */
>> > chan->psm = psm;
>> > chan->dcid = scid;
>> >
>> > The conn list reference taken at (A), before (C), is what keeps new_chan
>> > alive across the release_sock() window so l2cap_connect() can keep using it.
>> >
>> > So __l2cap_chan_add() can't move out to l2cap_new_connection(): by the time
>> > the callback returns, the parent lock is already dropped and chan may already
>> > be freed - which is exactly the race. It has to be taken inside the callback,
>> > under the parent lock, before the socket is exposed.
>
> If you do after l2cap_new_connection, yes, but what if you do before
> you call ->new_connect:
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 3991c179b98f..cd864f400985 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4027,8 +4027,10 @@ static struct l2cap_chan
> *l2cap_new_connection(struct l2cap_conn *conn,
> if (!chan)
> return NULL;
>
> + __l2cap_chan_add(conn, chan);
> +
> if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
> - l2cap_chan_put(chan);
> + l2cap_chan_del(chan, 0);
> return NULL;
> }
>
> This means we have to call l2cap_chan_del if new_connection fails, but
> we can eliminate other functions having to call __l2cap_chan_add with
> this. If the issue is then having l2cap_chan_del called concurrently,
> we can add a l2cap_chan_hold; actually, we may need a reference anyway
> since sk can be released and call l2cap_chan_del, releasing both
> references.
>
In:
void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
{
...
switch (chan->chan_type) {
case L2CAP_CHAN_CONN_ORIENTED:
...
break;
case L2CAP_CHAN_CONN_LESS:
...
break;
case L2CAP_CHAN_FIXED:
...
break;
default:
...
}
}
Apparently it depends on chan->chan_type and it is set by ref1.
It is only set in cb function.
>> > The other callbacks (6lowpan, smp) have no equivalent lock-drop window.
>> > I kept __l2cap_chan_add() inside all of the ->new_connection() callbacks
>> > just to keep the "callback links the channel into conn" contract uniform.
>> >
>>
>> Could you please check this?
>> Just remind you in case you miss this.
>>
>> >>> +
>> >>> + return chan;
>> >>> +}
>> >>> +
>> >>> static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>> >>> u8 *data, u8 rsp_code)
>> >>> {
>> >>> @@ -4053,7 +4078,7 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>> >>> goto response;
>> >>> }
>> >>>
>> >>> - chan = pchan->ops->new_connection(pchan);
>> >>> + chan = l2cap_new_connection(conn, pchan);
>> >>> if (!chan)
>> >>> goto response;
>> >>>
>> >>> @@ -4071,8 +4096,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>> >>> chan->psm = psm;
>> >>> chan->dcid = scid;
>> >>>
>> >>> - __l2cap_chan_add(conn, chan);
>> >>> -
>> >>> dcid = chan->scid;
>> >>>
>> >>> __set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
>> >>> @@ -4955,7 +4978,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
>> >>> goto response_unlock;
>> >>> }
>> >>>
>> >>> - chan = pchan->ops->new_connection(pchan);
>> >>> + chan = l2cap_new_connection(conn, pchan);
>> >>> if (!chan) {
>> >>> result = L2CAP_CR_LE_NO_MEM;
>> >>> goto response_unlock;
>> >>> @@ -4970,8 +4993,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
>> >>> chan->omtu = mtu;
>> >>> chan->remote_mps = mps;
>> >>>
>> >>> - __l2cap_chan_add(conn, chan);
>> >>> -
>> >>> l2cap_le_flowctl_init(chan, __le16_to_cpu(req->credits));
>> >>>
>> >>> dcid = chan->scid;
>> >>> @@ -5179,7 +5200,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>> >>> continue;
>> >>> }
>> >>>
>> >>> - chan = pchan->ops->new_connection(pchan);
>> >>> + chan = l2cap_new_connection(conn, pchan);
>> >>> if (!chan) {
>> >>> result = L2CAP_CR_LE_NO_MEM;
>> >>> continue;
>> >>> @@ -5194,8 +5215,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>> >>> chan->omtu = mtu;
>> >>> chan->remote_mps = mps;
>> >>>
>> >>> - __l2cap_chan_add(conn, chan);
>> >>> -
>> >>> l2cap_ecred_init(chan, __le16_to_cpu(req->credits));
>> >>>
>> >>> /* Init response */
>> >>> @@ -7470,14 +7489,12 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
>> >>> goto next;
>> >>>
>> >>> l2cap_chan_lock(pchan);
>> >>> - chan = pchan->ops->new_connection(pchan);
>> >>> + chan = l2cap_new_connection(conn, pchan);
>> >>> if (chan) {
>> >>> bacpy(&chan->src, &hcon->src);
>> >>> bacpy(&chan->dst, &hcon->dst);
>> >>> chan->src_type = bdaddr_src_type(hcon);
>> >>> chan->dst_type = dst_type;
>> >>> -
>> >>> - __l2cap_chan_add(conn, chan);
>> >>> }
>> >>>
>> >>> l2cap_chan_unlock(pchan);
>> >>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> >>> index 025329636353..87f4c0db5c0c 100644
>> >>> --- a/net/bluetooth/l2cap_sock.c
>> >>> +++ b/net/bluetooth/l2cap_sock.c
>> >>> @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = {
>> >>> static const struct proto_ops l2cap_sock_ops;
>> >>> static void l2cap_sock_init(struct sock *sk, struct sock *parent);
>> >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
>> >>> - int proto, gfp_t prio, int kern);
>> >>> + int proto, gfp_t prio, int kern,
>> >>> + struct l2cap_chan *chan);
>> >>> static void l2cap_sock_cleanup_listen(struct sock *parent);
>> >>>
>> >>> bool l2cap_is_socket(struct socket *sock)
>> >>> @@ -1287,6 +1288,23 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
>> >>> return err;
>> >>> }
>> >>>
>> >>> +/* Release the sock's ref on chan and clear the pointer so that the ref is
>> >>> + * dropped exactly once even if both l2cap_sock_kill() and
>> >>> + * l2cap_sock_destruct() run. Setting chan->data to NULL first stops any other
>> >>> + * task from dereferencing the now-dead sock pointer.
>> >>> + */
>> >>> +static void l2cap_sock_put_chan(struct sock *sk)
>> >>> +{
>> >>> + struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>> >>> +
>> >>> + if (!chan)
>> >>> + return;
>> >>> +
>> >>> + chan->data = NULL;
>> >>> + l2cap_pi(sk)->chan = NULL;
>> >>> + l2cap_chan_put(chan);
>> >>> +}
>> >>> +
>> >>> /* Kill socket (only if zapped and orphan)
>> >>> * Must be called on unlocked socket, with l2cap channel lock.
>> >>> */
>> >>> @@ -1297,13 +1315,9 @@ static void l2cap_sock_kill(struct sock *sk)
>> >>>
>> >>> BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
>> >>>
>> >>> - /* Sock is dead, so set chan data to NULL, avoid other task use invalid
>> >>> - * sock pointer.
>> >>> - */
>> >>> - l2cap_pi(sk)->chan->data = NULL;
>> >>> - /* Kill poor orphan */
>> >>> + l2cap_sock_put_chan(sk);
>> >>>
>> >>> - l2cap_chan_put(l2cap_pi(sk)->chan);
>> >>> + /* Kill poor orphan */
>> >>> sock_set_flag(sk, SOCK_DEAD);
>> >>> sock_put(sk);
>> >>> }
>> >>> @@ -1546,12 +1560,14 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
>> >>> }
>> >>> }
>> >>>
>> >>> -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>> >>> +static int l2cap_sock_new_connection_cb(struct l2cap_conn *conn,
>> >>> + struct l2cap_chan *chan,
>> >>> + struct l2cap_chan *new_chan)
>> >>> {
>> >>> struct sock *sk, *parent = chan->data;
>> >>>
>> >>> if (!parent)
>> >>> - return NULL;
>> >>> + return -EINVAL;
>> >>>
>> >>> lock_sock(parent);
>> >>>
>> >>> @@ -1559,25 +1575,33 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>> >>> if (sk_acceptq_is_full(parent)) {
>> >>> BT_DBG("backlog full %d", parent->sk_ack_backlog);
>> >>> release_sock(parent);
>> >>> - return NULL;
>> >>> + return -ENOBUFS;
>> >>> }
>> >>>
>> >>> sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
>> >>> - GFP_ATOMIC, 0);
>> >>> + GFP_ATOMIC, 0, new_chan);
>> >>> if (!sk) {
>> >>> release_sock(parent);
>> >>> - return NULL;
>> >>> - }
>> >>> + return -ENOMEM;
>> >>> + }
>> >>>
>> >>> bt_sock_reclassify_lock(sk, BTPROTO_L2CAP);
>> >>>
>> >>> l2cap_sock_init(sk, parent);
>> >>>
>> >>> + /* Link the channel into conn before exposing the new socket via the
>> >>> + * accept queue. Once release_sock() below drops the parent lock the
>> >>> + * socket may be freed by another task, dropping its reference on
>> >>> + * new_chan; the conn list reference taken here keeps new_chan alive so
>> >>> + * the caller can safely use it after ->new_connection() returns.
>> >>> + */
>> >>> + __l2cap_chan_add(conn, new_chan);
>> >>> +
>> >>> bt_accept_enqueue(parent, sk, false);
>> >>>
>> >>> release_sock(parent);
>> >>>
>> >>> - return l2cap_pi(sk)->chan;
>> >>> + return 0;
>> >>> }
>> >>>
>> >>> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
>> >>> @@ -1874,10 +1898,7 @@ static void l2cap_sock_destruct(struct sock *sk)
>> >>>
>> >>> BT_DBG("sk %p", sk);
>> >>>
>> >>> - if (l2cap_pi(sk)->chan) {
>> >>> - l2cap_pi(sk)->chan->data = NULL;
>> >>> - l2cap_chan_put(l2cap_pi(sk)->chan);
>> >>> - }
>> >>> + l2cap_sock_put_chan(sk);
>> >>>
>> >>> list_for_each_entry_safe(rx_busy, next, &l2cap_pi(sk)->rx_busy, list) {
>> >>> kfree_skb(rx_busy->skb);
>> >>> @@ -1978,10 +1999,10 @@ static struct proto l2cap_proto = {
>> >>> };
>> >>>
>> >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
>> >>> - int proto, gfp_t prio, int kern)
>> >>> + int proto, gfp_t prio, int kern,
>> >>> + struct l2cap_chan *chan)
>> >>> {
>> >>> struct sock *sk;
>> >>> - struct l2cap_chan *chan;
>> >>>
>> >>> sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern);
>> >>> if (!sk)
>> >>> @@ -1992,16 +2013,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
>> >>>
>> >>> INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy);
>> >>>
>> >>> - chan = l2cap_chan_create();
>> >>> - if (!chan) {
>> >>> - sk_free(sk);
>> >>> - if (sock)
>> >>> - sock->sk = NULL;
>> >>> - return NULL;
>> >>> - }
>> >>> -
>> >>> - l2cap_chan_hold(chan);
>> >>> -
>> >>> + /* The sock takes ownership of the caller's reference on chan. */
>> >>> l2cap_pi(sk)->chan = chan;
>> >>>
>> >>> return sk;
>> >>> @@ -2011,6 +2023,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol,
>> >>> int kern)
>> >>> {
>> >>> struct sock *sk;
>> >>> + struct l2cap_chan *chan;
>> >>>
>> >>> BT_DBG("sock %p", sock);
>> >>>
>> >>> @@ -2025,10 +2038,16 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol,
>> >>>
>> >>> sock->ops = &l2cap_sock_ops;
>> >>>
>> >>> - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern);
>> >>> - if (!sk)
>> >>> + chan = l2cap_chan_create();
>> >>> + if (!chan)
>> >>> return -ENOMEM;
>> >>>
>> >>> + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan);
>> >>> + if (!sk) {
>> >>> + l2cap_chan_put(chan);
>> >>> + return -ENOMEM;
>> >>> + }
>> >>> +
>> >>> l2cap_sock_init(sk, NULL);
>> >>> bt_sock_link(&l2cap_sk_list, sk);
>> >>> return 0;
>> >>> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
>> >>> index 1739c1989dbd..2d31c3c7bbc0 100644
>> >>> --- a/net/bluetooth/smp.c
>> >>> +++ b/net/bluetooth/smp.c
>> >>> @@ -3204,16 +3204,12 @@ static const struct l2cap_ops smp_chan_ops = {
>> >>> .get_sndtimeo = l2cap_chan_no_get_sndtimeo,
>> >>> };
>> >>>
>> >>> -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan)
>> >>> +static inline int smp_new_conn_cb(struct l2cap_conn *conn,
>> >>> + struct l2cap_chan *pchan,
>> >>> + struct l2cap_chan *chan)
>> >>> {
>> >>> - struct l2cap_chan *chan;
>> >>> -
>> >>> BT_DBG("pchan %p", pchan);
>> >>>
>> >>> - chan = l2cap_chan_create();
>> >>> - if (!chan)
>> >>> - return NULL;
>> >>> -
>> >>> chan->chan_type = pchan->chan_type;
>> >>> chan->ops = &smp_chan_ops;
>> >>> chan->scid = pchan->scid;
>> >>> @@ -3229,9 +3225,12 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan)
>> >>> */
>> >>> atomic_set(&chan->nesting, L2CAP_NESTING_SMP);
>> >>>
>> >>> - BT_DBG("created chan %p", chan);
>> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
>> >>> + __l2cap_chan_add(conn, chan);
>> >>>
>> >>> - return chan;
>> >>> + BT_DBG("initialised chan %p", chan);
>> >>> +
>> >>> + return 0;
>> >>> }
>> >>>
>> >>> static const struct l2cap_ops smp_root_chan_ops = {
>> >>> --
>> >>> 2.54.0
>> >>>
>> >>
>> >>
>> >> --
>> >> Luiz Augusto von Dentz
>> >
>> > Best,
>> > Siwei
>>
>> Best,
>> Siwei
>
>
>
> --
> Luiz Augusto von Dentz
Best,
Siwei
^ permalink raw reply
* [PATCH v2] Bluetooth: btintel_pcie: Separate coredump work from RX work
From: Kiran K @ 2026-06-10 16:25 UTC (permalink / raw)
To: linux-bluetooth
Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
chandrashekar.devegowda, ravindra, Kiran K
From: Ravindra <ravindra@intel.com>
Sharing a single workqueue between coredump processing and RX
delays evacuation of RX events while a coredump is in progress.
The firmware's RX buffers can overflow during that window, leading
to dropped events. The issue was observed in HID use cases where
HID reports arrive in bursts and quickly fill the RX path while a
coredump is being collected.
Move coredump processing to a dedicated ordered coredump_workqueue
with its own coredump_work, so coredumps run independently of RX.
All four coredump trigger sources (FW assert, HW exception, user
sysfs trigger, and resume-error detection) are switched to this new
workqueue. Ordering serialises concurrent triggers without blocking
RX.
Signed-off-by: Ravindra <ravindra@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
changes in v2:
- Fix the race condition reported by Sashiko b/w reset_work() and
.remove()
drivers/bluetooth/btintel_pcie.c | 102 +++++++++++++++++++++++++------
drivers/bluetooth/btintel_pcie.h | 5 ++
2 files changed, 87 insertions(+), 20 deletions(-)
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 52293d19c817..e2f64d42c11c 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -1457,7 +1457,7 @@ static void btintel_pcie_msix_fw_trigger_handler(struct btintel_pcie_data *data)
if (!test_and_set_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags))
data->dmp_hdr.trigger_reason = BTINTEL_PCIE_TRIGGER_REASON_FW_ASSERT;
- queue_work(data->workqueue, &data->rx_work);
+ queue_work(data->coredump_workqueue, &data->coredump_work);
}
static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data *data)
@@ -1474,16 +1474,21 @@ static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data *data)
if (!test_and_set_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags))
data->dmp_hdr.trigger_reason = BTINTEL_PCIE_TRIGGER_REASON_FW_ASSERT;
- queue_work(data->workqueue, &data->rx_work);
+ queue_work(data->coredump_workqueue, &data->coredump_work);
}
-static void btintel_pcie_rx_work(struct work_struct *work)
+static void btintel_pcie_coredump_worker(struct work_struct *work)
{
struct btintel_pcie_data *data = container_of(work,
- struct btintel_pcie_data, rx_work);
- struct sk_buff *skb;
+ struct btintel_pcie_data, coredump_work);
int err;
+ /* hdev is NULL until setup_hdev() succeeds, and is cleared on
+ * teardown after disable_work_sync() drains us; bail in that case.
+ */
+ if (!data->hdev)
+ return;
+
if (test_bit(BTINTEL_PCIE_FWTRIGGER_DUMP_INPROGRESS, &data->flags)) {
err = btintel_pcie_dump_fwtrigger_event(data);
if (err)
@@ -1507,6 +1512,13 @@ static void btintel_pcie_rx_work(struct work_struct *work)
btintel_pcie_read_hwexp(data);
clear_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags);
}
+}
+
+static void btintel_pcie_rx_work(struct work_struct *work)
+{
+ struct btintel_pcie_data *data = container_of(work,
+ struct btintel_pcie_data, rx_work);
+ struct sk_buff *skb;
/* Process the sk_buf in queue and send to the HCI layer */
while ((skb = skb_dequeue(&data->rx_skb_q))) {
@@ -2181,9 +2193,11 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
static void btintel_pcie_release_hdev(struct btintel_pcie_data *data)
{
- struct hci_dev *hdev;
+ struct hci_dev *hdev = data->hdev;
+
+ if (!hdev)
+ return;
- hdev = data->hdev;
hci_unregister_dev(hdev);
hci_free_dev(hdev);
data->hdev = NULL;
@@ -2603,6 +2617,10 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
btintel_pcie_synchronize_irqs(data);
flush_work(&data->rx_work);
+ /* Drain any in-flight coredump and block new ones across reset.
+ * Safe from self-deadlock: coredump_work runs on a separate wq.
+ */
+ disable_work_sync(&data->coredump_work);
bt_dev_dbg(data->hdev, "Release bluetooth interface");
if (data->reset_type == BTINTEL_PCIE_IOSF_PRR_PLDR) {
@@ -2610,16 +2628,27 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
* pci_rescan_remove_lock. This mutex serializes against PCI device
* addition/removal (hotplug), so no device can be added to or
* removed from the bus list while this code runs.
+ *
+ * device_reprobe() inside btintel_pcie_perform_pldr() destroys
+ * 'data' via .remove(); a fresh probe re-INIT_WORKs the
+ * coredump_work with disable count 0, so we must not call
+ * enable_work() on this path.
*/
btintel_pcie_perform_pldr(data);
goto out;
}
btintel_pcie_release_hdev(data);
- err = pci_reset_function(pdev);
+ /* Use pci_try_reset_function() rather than pci_reset_function() to
+ * avoid an ABBA deadlock against btintel_pcie_remove(): the PCI core
+ * calls .remove() with device_lock held, and remove() then waits for
+ * this work via cancel_work_sync(); pci_reset_function() would in
+ * turn try to acquire the same device_lock, deadlocking both paths.
+ */
+ err = pci_try_reset_function(pdev);
if (err) {
BT_ERR("Failed resetting the pcie device (%d)", err);
- goto out;
+ goto out_enable;
}
btintel_pcie_enable_interrupts(data);
@@ -2629,7 +2658,7 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
if (err) {
BT_ERR("Failed to enable bluetooth hardware after reset (%d)",
err);
- goto out;
+ goto out_enable;
}
btintel_pcie_reset_ia(data);
@@ -2639,8 +2668,15 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
err = btintel_pcie_setup_hdev(data);
if (err) {
BT_ERR("Failed registering hdev (%d)", err);
- goto out;
+ goto out_enable;
}
+
+out_enable:
+ /* Balance disable_work_sync() above on every exit. Leaving the
+ * counter incremented on a failed reset would permanently disable
+ * coredump_work even after a later successful reset.
+ */
+ enable_work(&data->coredump_work);
out:
pci_dev_put(pdev);
pci_unlock_rescan_remove();
@@ -2774,7 +2810,6 @@ static int btintel_pcie_setup_hdev(struct btintel_pcie_data *data)
hdev->bus = HCI_PCI;
hci_set_drvdata(hdev, data);
- data->hdev = hdev;
SET_HCIDEV_DEV(hdev, &data->pdev->dev);
hdev->manufacturer = 2;
@@ -2793,15 +2828,17 @@ static int btintel_pcie_setup_hdev(struct btintel_pcie_data *data)
err = hci_register_dev(hdev);
if (err < 0) {
BT_ERR("Failed to register to hdev (%d)", err);
- goto exit_error;
+ hci_free_dev(hdev);
+ return err;
}
+ /* Publish hdev only after successful registration; the coredump
+ * worker bails on !data->hdev, so it never observes a half-set-up
+ * device.
+ */
+ data->hdev = hdev;
data->dmp_hdr.driver_name = KBUILD_MODNAME;
return 0;
-
-exit_error:
- hci_free_dev(hdev);
- return err;
}
static int btintel_pcie_probe(struct pci_dev *pdev,
@@ -2832,9 +2869,16 @@ static int btintel_pcie_probe(struct pci_dev *pdev,
if (!data->workqueue)
return -ENOMEM;
+ data->coredump_workqueue = alloc_ordered_workqueue(KBUILD_MODNAME "_cd", 0);
+ if (!data->coredump_workqueue) {
+ destroy_workqueue(data->workqueue);
+ return -ENOMEM;
+ }
+
skb_queue_head_init(&data->rx_skb_q);
INIT_WORK(&data->rx_work, btintel_pcie_rx_work);
INIT_WORK(&data->reset_work, btintel_pcie_reset_work);
+ INIT_WORK(&data->coredump_work, btintel_pcie_coredump_worker);
data->boot_stage_cache = 0x00;
data->img_resp_cache = 0x00;
@@ -2877,6 +2921,8 @@ static int btintel_pcie_probe(struct pci_dev *pdev,
/* reset device before exit */
btintel_pcie_reset_bt(data);
+ destroy_workqueue(data->coredump_workqueue);
+
pci_clear_master(pdev);
pci_set_drvdata(pdev, NULL);
@@ -2894,13 +2940,20 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
return;
}
+ /* Permanently block coredump triggers and drain the worker before
+ * tearing down. Must run before cancel_work_sync(&reset_work) so
+ * the disable counter stays >= 1 even after reset_work()'s
+ * balanced enable_work() (counter 2 -> 1, never reaching 0).
+ */
+ disable_work_sync(&data->coredump_work);
+
/* Cancel pending reset work. Skip only when remove() is called from
* within the reset work itself (PLDR device_reprobe path) to avoid
* deadlock. current_work() returns the work_struct of the caller if
* we are in a workqueue context.
*/
if (current_work() != &data->reset_work)
- cancel_work_sync(&data->reset_work);
+ disable_work_sync(&data->reset_work);
btintel_pcie_disable_interrupts(data);
@@ -2920,6 +2973,7 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
btintel_pcie_release_hdev(data);
+ destroy_workqueue(data->coredump_workqueue);
destroy_workqueue(data->workqueue);
btintel_pcie_free(data);
@@ -2935,11 +2989,19 @@ static void btintel_pcie_coredump(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
struct btintel_pcie_data *data = pci_get_drvdata(pdev);
+ if (!data)
+ return;
+
if (test_and_set_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags))
return;
data->dmp_hdr.trigger_reason = BTINTEL_PCIE_TRIGGER_REASON_USER_TRIGGER;
- queue_work(data->workqueue, &data->rx_work);
+ /* queue_work() returns false if the work is disabled (reset or
+ * remove in progress); clear the in-progress bit so a later
+ * trigger can succeed once the work is re-enabled.
+ */
+ if (!queue_work(data->coredump_workqueue, &data->coredump_work))
+ clear_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags);
}
#endif
@@ -3080,7 +3142,7 @@ static int btintel_pcie_resume(struct device *dev)
&data->flags)) {
data->dmp_hdr.trigger_reason =
BTINTEL_PCIE_TRIGGER_REASON_FW_ASSERT;
- queue_work(data->workqueue, &data->rx_work);
+ queue_work(data->coredump_workqueue, &data->coredump_work);
}
set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
btintel_pcie_reset(data->hdev);
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index e4a8fa479188..7caee093e316 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -466,6 +466,8 @@ struct btintel_pcie_dump_header {
* @workqueue: workqueue for RX work
* @rx_skb_q: SKB queue for RX packet
* @rx_work: RX work struct to process the RX packet in @rx_skb_q
+ * @coredump_workqueue: dedicated workqueue for coredump collection
+ * @coredump_work: work struct for coredump trace collection
* @dma_pool: DMA pool for descriptors, index array and ci
* @dma_p_addr: DMA address for pool
* @dma_v_addr: address of pool
@@ -514,6 +516,9 @@ struct btintel_pcie_data {
struct work_struct rx_work;
struct work_struct reset_work;
+ struct workqueue_struct *coredump_workqueue;
+ struct work_struct coredump_work;
+
struct dma_pool *dma_pool;
dma_addr_t dma_p_addr;
void *dma_v_addr;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v9 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()
From: Luiz Augusto von Dentz @ 2026-06-10 15:57 UTC (permalink / raw)
To: Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <13e2aafc-8a4c-4c00-b38a-b09deda670ce@app.fastmail.com>
Hi Siwei,
On Mon, Jun 8, 2026 at 9:29 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>
> Hi Luiz,
>
> On Thu, Jun 4, 2026, at 11:52 AM, Siwei Zhang wrote:
> > Hi Luiz,
> >
> > On Wed, Jun 3, 2026, at 2:17 PM, Luiz Augusto von Dentz wrote:
> >> Hi Siwei,
> >>
> >> On Wed, Jun 3, 2026 at 11:09 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >>>
> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
> >>> release_sock(parent). Once the parent lock is released, the child
> >>> socket sk can be freed by another task.
> >>>
> >>> Allocate the channel outside the func to prevent this.
> >>>
> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
> >>> Cc: stable@kernel.org
> >>> Assisted-by: Claude:claude-opus-4-8
> >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> >>> ---
> >>> include/net/bluetooth/l2cap.h | 10 +++--
> >>> net/bluetooth/6lowpan.c | 31 +++++++------
> >>> net/bluetooth/l2cap_core.c | 41 ++++++++++++-----
> >>> net/bluetooth/l2cap_sock.c | 83 +++++++++++++++++++++--------------
> >>> net/bluetooth/smp.c | 17 ++++---
> >>> 5 files changed, 113 insertions(+), 69 deletions(-)
> >>>
> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >>> index e0a1f2293679..7f5e4647f6e0 100644
> >>> --- a/include/net/bluetooth/l2cap.h
> >>> +++ b/include/net/bluetooth/l2cap.h
> >>> @@ -620,7 +620,9 @@ struct l2cap_chan {
> >>> struct l2cap_ops {
> >>> char *name;
> >>>
> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
> >>> + int (*new_connection)(struct l2cap_conn *conn,
> >>> + struct l2cap_chan *chan,
> >>> + struct l2cap_chan *new_chan);
> >>> int (*recv) (struct l2cap_chan * chan,
> >>> struct sk_buff *skb);
> >>> void (*teardown) (struct l2cap_chan *chan, int err);
> >>> @@ -884,9 +886,11 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
> >>> return (seq + 1) % (chan->tx_win_max + 1);
> >>> }
> >>>
> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_conn *conn,
> >>> + struct l2cap_chan *chan,
> >>> + struct l2cap_chan *new_chan)
> >>> {
> >>> - return NULL;
> >>> + return -EOPNOTSUPP;
> >>> }
> >>>
> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb)
> >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> >>> index cb1e329d66fd..94863af97a44 100644
> >>> --- a/net/bluetooth/6lowpan.c
> >>> +++ b/net/bluetooth/6lowpan.c
> >>> @@ -624,6 +624,15 @@ static bool is_bt_6lowpan(struct hci_conn *hcon)
> >>> return true;
> >>> }
> >>>
> >>> +static void chan_init(struct l2cap_chan *chan)
> >>> +{
> >>> + l2cap_chan_set_defaults(chan);
> >>> +
> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >>> + chan->imtu = 1280;
> >>> +}
> >>> +
> >>> static struct l2cap_chan *chan_create(void)
> >>> {
> >>> struct l2cap_chan *chan;
> >>> @@ -632,11 +641,7 @@ static struct l2cap_chan *chan_create(void)
> >>> if (!chan)
> >>> return NULL;
> >>>
> >>> - l2cap_chan_set_defaults(chan);
> >>> -
> >>> - chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> >>> - chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >>> - chan->imtu = 1280;
> >>> + chan_init(chan);
> >>>
> >>> return chan;
> >>> }
> >>> @@ -745,19 +750,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan)
> >>> ifup(dev->netdev);
> >>> }
> >>>
> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
> >>> +static inline int chan_new_conn_cb(struct l2cap_conn *conn,
> >>> + struct l2cap_chan *pchan,
> >>> + struct l2cap_chan *chan)
> >>> {
> >>> - struct l2cap_chan *chan;
> >>> -
> >>> - chan = chan_create();
> >>> - if (!chan)
> >>> - return NULL;
> >>> -
> >>> + chan_init(chan);
> >>> chan->ops = pchan->ops;
> >>>
> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
> >>> + __l2cap_chan_add(conn, chan);
> >>> +
> >>> BT_DBG("chan %p pchan %p", chan, pchan);
> >>>
> >>> - return chan;
> >>> + return 0;
> >>> }
> >>>
> >>> static void unregister_dev(struct lowpan_btle_dev *dev)
> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>> index c4ccfbda9d78..62acf90837fb 100644
> >>> --- a/net/bluetooth/l2cap_core.c
> >>> +++ b/net/bluetooth/l2cap_core.c
> >>> @@ -4007,6 +4007,31 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
> >>> return 0;
> >>> }
> >>>
> >>> +/* Allocate and initialise a channel for an incoming connection.
> >>> + *
> >>> + * ->new_connection() initialises the channel and links it into @conn with
> >>> + * __l2cap_chan_add(). The l2cap_chan_create() reference becomes the one owned
> >>> + * by the parent subsystem (l2cap_pi(sk)->chan, conn->smp or peer->chan) and is
> >>> + * released by its teardown callback; the conn list reference is released by
> >>> + * l2cap_chan_del().
> >>> + */
> >>> +static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
> >>> + struct l2cap_chan *pchan)
> >>> +{
> >>> + struct l2cap_chan *chan;
> >>> +
> >>> + chan = l2cap_chan_create();
> >>> + if (!chan)
> >>> + return NULL;
> >>> +
> >>> + if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
> >>> + l2cap_chan_put(chan);
> >>> + return NULL;
> >>> + }
> >>
> >> I don't quite get why we can't just place __l2cap_chan_add here
> >> instead of having it called by new_connection callbacks?
> >>
> >
> > It's specifically the l2cap_sock_new_connection_cb() case - the very
> > use-after-free this patch fixes. The __l2cap_chan_add() has to happen while
> > the parent lock is still held, and only the callback holds that lock.
> >
> > The reference counting on the new child chan starts at one ref,
> > owned by the new socket:
> >
> > /* l2cap_new_connection() */
> > chan = l2cap_chan_create(); /* refcount = 1 */
> > if (!chan)
> > return NULL;
> >
> > pchan->ops->new_connection(conn, pchan, chan);
> >
> > and inside the socket callback:
> >
> > /* l2cap_sock_new_connection_cb() */
> > lock_sock(parent);
> > ...
> > sk = l2cap_sock_alloc(..., new_chan); /* sk owns the chan_create ref */
> > ...
> > l2cap_sock_init(sk, parent);
> >
> > __l2cap_chan_add(conn, new_chan); /* (A) conn list takes a ref */
> > bt_accept_enqueue(parent, sk, false); /* (B) sk now on accept queue */
> >
> > release_sock(parent); /* (C) parent lock dropped */
> > return 0;
> >
> > The moment we hit (C), sk is reachable through the parent's accept queue, so
> > another task can grab and tear it down:
> >
> > accept() -> l2cap_sock_kill() -> l2cap_sock_put_chan()
> > chan->data = NULL;
> > l2cap_chan_put(chan); /* drops the sk's chan ref */
> >
> > If __l2cap_chan_add() at (A) hadn't already taken the conn list reference,
> > that put would drop the last ref and free new_chan. Control then returns up
> > to l2cap_new_connection(), which hands the now-freed chan back to
> > l2cap_connect():
> >
> > /* l2cap_connect() - runs after the callback returns */
> > chan = l2cap_new_connection(conn, pchan);
> > if (!chan)
> > goto response;
> > ...
> > bacpy(&chan->src, &conn->hcon->src); /* <-- UAF on freed chan */
> > chan->psm = psm;
> > chan->dcid = scid;
> >
> > The conn list reference taken at (A), before (C), is what keeps new_chan
> > alive across the release_sock() window so l2cap_connect() can keep using it.
> >
> > So __l2cap_chan_add() can't move out to l2cap_new_connection(): by the time
> > the callback returns, the parent lock is already dropped and chan may already
> > be freed - which is exactly the race. It has to be taken inside the callback,
> > under the parent lock, before the socket is exposed.
If you do after l2cap_new_connection, yes, but what if you do before
you call ->new_connect:
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3991c179b98f..cd864f400985 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4027,8 +4027,10 @@ static struct l2cap_chan
*l2cap_new_connection(struct l2cap_conn *conn,
if (!chan)
return NULL;
+ __l2cap_chan_add(conn, chan);
+
if (pchan->ops->new_connection(conn, pchan, chan) < 0) {
- l2cap_chan_put(chan);
+ l2cap_chan_del(chan, 0);
return NULL;
}
This means we have to call l2cap_chan_del if new_connection fails, but
we can eliminate other functions having to call __l2cap_chan_add with
this. If the issue is then having l2cap_chan_del called concurrently,
we can add a l2cap_chan_hold; actually, we may need a reference anyway
since sk can be released and call l2cap_chan_del, releasing both
references.
> > The other callbacks (6lowpan, smp) have no equivalent lock-drop window.
> > I kept __l2cap_chan_add() inside all of the ->new_connection() callbacks
> > just to keep the "callback links the channel into conn" contract uniform.
> >
>
> Could you please check this?
> Just remind you in case you miss this.
>
> >>> +
> >>> + return chan;
> >>> +}
> >>> +
> >>> static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> >>> u8 *data, u8 rsp_code)
> >>> {
> >>> @@ -4053,7 +4078,7 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> >>> goto response;
> >>> }
> >>>
> >>> - chan = pchan->ops->new_connection(pchan);
> >>> + chan = l2cap_new_connection(conn, pchan);
> >>> if (!chan)
> >>> goto response;
> >>>
> >>> @@ -4071,8 +4096,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> >>> chan->psm = psm;
> >>> chan->dcid = scid;
> >>>
> >>> - __l2cap_chan_add(conn, chan);
> >>> -
> >>> dcid = chan->scid;
> >>>
> >>> __set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
> >>> @@ -4955,7 +4978,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> >>> goto response_unlock;
> >>> }
> >>>
> >>> - chan = pchan->ops->new_connection(pchan);
> >>> + chan = l2cap_new_connection(conn, pchan);
> >>> if (!chan) {
> >>> result = L2CAP_CR_LE_NO_MEM;
> >>> goto response_unlock;
> >>> @@ -4970,8 +4993,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> >>> chan->omtu = mtu;
> >>> chan->remote_mps = mps;
> >>>
> >>> - __l2cap_chan_add(conn, chan);
> >>> -
> >>> l2cap_le_flowctl_init(chan, __le16_to_cpu(req->credits));
> >>>
> >>> dcid = chan->scid;
> >>> @@ -5179,7 +5200,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> >>> continue;
> >>> }
> >>>
> >>> - chan = pchan->ops->new_connection(pchan);
> >>> + chan = l2cap_new_connection(conn, pchan);
> >>> if (!chan) {
> >>> result = L2CAP_CR_LE_NO_MEM;
> >>> continue;
> >>> @@ -5194,8 +5215,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> >>> chan->omtu = mtu;
> >>> chan->remote_mps = mps;
> >>>
> >>> - __l2cap_chan_add(conn, chan);
> >>> -
> >>> l2cap_ecred_init(chan, __le16_to_cpu(req->credits));
> >>>
> >>> /* Init response */
> >>> @@ -7470,14 +7489,12 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
> >>> goto next;
> >>>
> >>> l2cap_chan_lock(pchan);
> >>> - chan = pchan->ops->new_connection(pchan);
> >>> + chan = l2cap_new_connection(conn, pchan);
> >>> if (chan) {
> >>> bacpy(&chan->src, &hcon->src);
> >>> bacpy(&chan->dst, &hcon->dst);
> >>> chan->src_type = bdaddr_src_type(hcon);
> >>> chan->dst_type = dst_type;
> >>> -
> >>> - __l2cap_chan_add(conn, chan);
> >>> }
> >>>
> >>> l2cap_chan_unlock(pchan);
> >>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> >>> index 025329636353..87f4c0db5c0c 100644
> >>> --- a/net/bluetooth/l2cap_sock.c
> >>> +++ b/net/bluetooth/l2cap_sock.c
> >>> @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = {
> >>> static const struct proto_ops l2cap_sock_ops;
> >>> static void l2cap_sock_init(struct sock *sk, struct sock *parent);
> >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
> >>> - int proto, gfp_t prio, int kern);
> >>> + int proto, gfp_t prio, int kern,
> >>> + struct l2cap_chan *chan);
> >>> static void l2cap_sock_cleanup_listen(struct sock *parent);
> >>>
> >>> bool l2cap_is_socket(struct socket *sock)
> >>> @@ -1287,6 +1288,23 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
> >>> return err;
> >>> }
> >>>
> >>> +/* Release the sock's ref on chan and clear the pointer so that the ref is
> >>> + * dropped exactly once even if both l2cap_sock_kill() and
> >>> + * l2cap_sock_destruct() run. Setting chan->data to NULL first stops any other
> >>> + * task from dereferencing the now-dead sock pointer.
> >>> + */
> >>> +static void l2cap_sock_put_chan(struct sock *sk)
> >>> +{
> >>> + struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> >>> +
> >>> + if (!chan)
> >>> + return;
> >>> +
> >>> + chan->data = NULL;
> >>> + l2cap_pi(sk)->chan = NULL;
> >>> + l2cap_chan_put(chan);
> >>> +}
> >>> +
> >>> /* Kill socket (only if zapped and orphan)
> >>> * Must be called on unlocked socket, with l2cap channel lock.
> >>> */
> >>> @@ -1297,13 +1315,9 @@ static void l2cap_sock_kill(struct sock *sk)
> >>>
> >>> BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
> >>>
> >>> - /* Sock is dead, so set chan data to NULL, avoid other task use invalid
> >>> - * sock pointer.
> >>> - */
> >>> - l2cap_pi(sk)->chan->data = NULL;
> >>> - /* Kill poor orphan */
> >>> + l2cap_sock_put_chan(sk);
> >>>
> >>> - l2cap_chan_put(l2cap_pi(sk)->chan);
> >>> + /* Kill poor orphan */
> >>> sock_set_flag(sk, SOCK_DEAD);
> >>> sock_put(sk);
> >>> }
> >>> @@ -1546,12 +1560,14 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
> >>> }
> >>> }
> >>>
> >>> -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
> >>> +static int l2cap_sock_new_connection_cb(struct l2cap_conn *conn,
> >>> + struct l2cap_chan *chan,
> >>> + struct l2cap_chan *new_chan)
> >>> {
> >>> struct sock *sk, *parent = chan->data;
> >>>
> >>> if (!parent)
> >>> - return NULL;
> >>> + return -EINVAL;
> >>>
> >>> lock_sock(parent);
> >>>
> >>> @@ -1559,25 +1575,33 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
> >>> if (sk_acceptq_is_full(parent)) {
> >>> BT_DBG("backlog full %d", parent->sk_ack_backlog);
> >>> release_sock(parent);
> >>> - return NULL;
> >>> + return -ENOBUFS;
> >>> }
> >>>
> >>> sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
> >>> - GFP_ATOMIC, 0);
> >>> + GFP_ATOMIC, 0, new_chan);
> >>> if (!sk) {
> >>> release_sock(parent);
> >>> - return NULL;
> >>> - }
> >>> + return -ENOMEM;
> >>> + }
> >>>
> >>> bt_sock_reclassify_lock(sk, BTPROTO_L2CAP);
> >>>
> >>> l2cap_sock_init(sk, parent);
> >>>
> >>> + /* Link the channel into conn before exposing the new socket via the
> >>> + * accept queue. Once release_sock() below drops the parent lock the
> >>> + * socket may be freed by another task, dropping its reference on
> >>> + * new_chan; the conn list reference taken here keeps new_chan alive so
> >>> + * the caller can safely use it after ->new_connection() returns.
> >>> + */
> >>> + __l2cap_chan_add(conn, new_chan);
> >>> +
> >>> bt_accept_enqueue(parent, sk, false);
> >>>
> >>> release_sock(parent);
> >>>
> >>> - return l2cap_pi(sk)->chan;
> >>> + return 0;
> >>> }
> >>>
> >>> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
> >>> @@ -1874,10 +1898,7 @@ static void l2cap_sock_destruct(struct sock *sk)
> >>>
> >>> BT_DBG("sk %p", sk);
> >>>
> >>> - if (l2cap_pi(sk)->chan) {
> >>> - l2cap_pi(sk)->chan->data = NULL;
> >>> - l2cap_chan_put(l2cap_pi(sk)->chan);
> >>> - }
> >>> + l2cap_sock_put_chan(sk);
> >>>
> >>> list_for_each_entry_safe(rx_busy, next, &l2cap_pi(sk)->rx_busy, list) {
> >>> kfree_skb(rx_busy->skb);
> >>> @@ -1978,10 +1999,10 @@ static struct proto l2cap_proto = {
> >>> };
> >>>
> >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
> >>> - int proto, gfp_t prio, int kern)
> >>> + int proto, gfp_t prio, int kern,
> >>> + struct l2cap_chan *chan)
> >>> {
> >>> struct sock *sk;
> >>> - struct l2cap_chan *chan;
> >>>
> >>> sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern);
> >>> if (!sk)
> >>> @@ -1992,16 +2013,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
> >>>
> >>> INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy);
> >>>
> >>> - chan = l2cap_chan_create();
> >>> - if (!chan) {
> >>> - sk_free(sk);
> >>> - if (sock)
> >>> - sock->sk = NULL;
> >>> - return NULL;
> >>> - }
> >>> -
> >>> - l2cap_chan_hold(chan);
> >>> -
> >>> + /* The sock takes ownership of the caller's reference on chan. */
> >>> l2cap_pi(sk)->chan = chan;
> >>>
> >>> return sk;
> >>> @@ -2011,6 +2023,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol,
> >>> int kern)
> >>> {
> >>> struct sock *sk;
> >>> + struct l2cap_chan *chan;
> >>>
> >>> BT_DBG("sock %p", sock);
> >>>
> >>> @@ -2025,10 +2038,16 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol,
> >>>
> >>> sock->ops = &l2cap_sock_ops;
> >>>
> >>> - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern);
> >>> - if (!sk)
> >>> + chan = l2cap_chan_create();
> >>> + if (!chan)
> >>> return -ENOMEM;
> >>>
> >>> + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan);
> >>> + if (!sk) {
> >>> + l2cap_chan_put(chan);
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> l2cap_sock_init(sk, NULL);
> >>> bt_sock_link(&l2cap_sk_list, sk);
> >>> return 0;
> >>> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> >>> index 1739c1989dbd..2d31c3c7bbc0 100644
> >>> --- a/net/bluetooth/smp.c
> >>> +++ b/net/bluetooth/smp.c
> >>> @@ -3204,16 +3204,12 @@ static const struct l2cap_ops smp_chan_ops = {
> >>> .get_sndtimeo = l2cap_chan_no_get_sndtimeo,
> >>> };
> >>>
> >>> -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan)
> >>> +static inline int smp_new_conn_cb(struct l2cap_conn *conn,
> >>> + struct l2cap_chan *pchan,
> >>> + struct l2cap_chan *chan)
> >>> {
> >>> - struct l2cap_chan *chan;
> >>> -
> >>> BT_DBG("pchan %p", pchan);
> >>>
> >>> - chan = l2cap_chan_create();
> >>> - if (!chan)
> >>> - return NULL;
> >>> -
> >>> chan->chan_type = pchan->chan_type;
> >>> chan->ops = &smp_chan_ops;
> >>> chan->scid = pchan->scid;
> >>> @@ -3229,9 +3225,12 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan)
> >>> */
> >>> atomic_set(&chan->nesting, L2CAP_NESTING_SMP);
> >>>
> >>> - BT_DBG("created chan %p", chan);
> >>> + /* Take the conn list reference; see l2cap_new_connection(). */
> >>> + __l2cap_chan_add(conn, chan);
> >>>
> >>> - return chan;
> >>> + BT_DBG("initialised chan %p", chan);
> >>> +
> >>> + return 0;
> >>> }
> >>>
> >>> static const struct l2cap_ops smp_root_chan_ops = {
> >>> --
> >>> 2.54.0
> >>>
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
> >
> > Best,
> > Siwei
>
> Best,
> Siwei
--
Luiz Augusto von Dentz
^ permalink raw reply related
* Re: [PATCH v2] Bluetooth: qca: Add BT FW build version to kernel log
From: patchwork-bot+bluetooth @ 2026-06-10 15:40 UTC (permalink / raw)
To: Xiuzhuo Shang
Cc: brgl, marcel, luiz.dentz, linux-arm-msm, linux-bluetooth,
linux-kernel, cheng.jiang, quic_chezhou, wei.deng, shuai.zhang,
mengshi.wu, jinwang.li, bartosz.golaszewski
In-Reply-To: <20260610064232.2385866-1-xiuzhuo.shang@oss.qualcomm.com>
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Wed, 10 Jun 2026 14:42:32 +0800 you wrote:
> Firmware version is critical for bug triage. Users reporting issues
> typically share dmesg output rather than debugfs contents, requiring
> extra communication rounds to collect this information. Log the FW
> build version directly to the kernel log so it is immediately
> available in bug reports.
>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Xiuzhuo Shang <xiuzhuo.shang@oss.qualcomm.com>
>
> [...]
Here is the summary with links:
- [v2] Bluetooth: qca: Add BT FW build version to kernel log
https://git.kernel.org/bluetooth/bluetooth-next/c/559cab24b04e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v1] Bluetooth: qca: Add BT FW build version log
From: patchwork-bot+bluetooth @ 2026-06-10 15:40 UTC (permalink / raw)
To: Xiuzhuo Shang
Cc: brgl, marcel, luiz.dentz, linux-arm-msm, linux-bluetooth,
linux-kernel, cheng.jiang, quic_chezhou, wei.deng, shuai.zhang,
mengshi.wu, jinwang.li
In-Reply-To: <20260609075417.1160702-1-xiuzhuo.shang@oss.qualcomm.com>
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Tue, 9 Jun 2026 15:54:17 +0800 you wrote:
> Printf BT FW build version log after BT FW downloaded.
>
> Signed-off-by: Xiuzhuo Shang <xiuzhuo.shang@oss.qualcomm.com>
> ---
> drivers/bluetooth/btqca.c | 2 ++
> 1 file changed, 2 insertions(+)
Here is the summary with links:
- [v1] Bluetooth: qca: Add BT FW build version log
https://git.kernel.org/bluetooth/bluetooth-next/c/559cab24b04e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] Bluetooth: L2CAP: validate connectionless PSM length
From: patchwork-bot+bluetooth @ 2026-06-10 15:40 UTC (permalink / raw)
To: Samuel Moelius; +Cc: marcel, luiz.dentz, linux-bluetooth, linux-kernel
In-Reply-To: <20260608235705.1233510.fe2269cf0103.bluetooth-l2cap-connless-short-pdu-oob@trailofbits.com>
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Mon, 8 Jun 2026 23:57:05 +0000 you wrote:
> Connectionless L2CAP frames carry a two-byte PSM at the start of the
> payload. l2cap_recv_frame() currently reads that PSM unconditionally
> after validating only the outer L2CAP length.
>
> A malformed connectionless frame with a zero- or one-byte payload can
> therefore make the parser read beyond the advertised skb payload and use
> tailroom bytes as part of the PSM. A VHCI-backed QEMU reproducer
> injected a one-byte connectionless payload and reached the unchecked
> read.
>
> [...]
Here is the summary with links:
- Bluetooth: L2CAP: validate connectionless PSM length
https://git.kernel.org/bluetooth/bluetooth-next/c/801f756504d1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox