* [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
* Re: [PATCH] Bluetooth: hci: validate codec capability element 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: <20260608235627.1233330.bc5338ecae62.bluetooth-hci-codec-cap-short-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:56:28 +0000 you wrote:
> Read Local Codec Capabilities returns a sequence of capability elements.
> Each element starts with a one-byte length followed by that many payload
> bytes.
>
> hci_read_codec_capabilities() checks that the skb contains the length
> byte, but then validates only caps->len against the remaining skb
> length. A malformed controller response with one remaining byte and
> caps->len set to one passes that check even though the element needs two
> bytes. The parser then records a two-byte capability and copies one
> byte beyond the advertised response payload into the codec list.
>
> [...]
Here is the summary with links:
- Bluetooth: hci: validate codec capability element length
https://git.kernel.org/bluetooth/bluetooth-next/c/246dc2ed724b
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 1/1] Bluetooth: hci_codec: validate capability record length
From: patchwork-bot+bluetooth @ 2026-06-10 15:40 UTC (permalink / raw)
To: Ren Wei
Cc: linux-bluetooth, marcel, luiz.dentz, kiran.k,
chethan.tumkur.narayan, ravishankar.srivatsa, yuantan098,
zcliangcn, bird, xuyq21
In-Reply-To: <4927cae4fe043f3e2aa80f4ee6bed05e4fb5a6d4.1779633761.git.xuyq21@lenovo.com>
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Fri, 29 May 2026 21:12:25 +0800 you wrote:
> From: Yuqi Xu <xuyq21@lenovo.com>
>
> hci_read_codec_capabilities() validates each capability entry before
> adding its serialized size to len and advancing the skb.
>
> The current check only compares skb->len against caps->len, even
> though the code consumes the length byte and the payload. Validate
> the full record size so the cached capability length matches the
> bytes available in the command response.
>
> [...]
Here is the summary with links:
- [1/1] Bluetooth: hci_codec: validate capability record length
https://git.kernel.org/bluetooth/bluetooth-next/c/246dc2ed724b
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: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Siwei Zhang @ 2026-06-10 15:34 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth, Luiz Augusto von Dentz
In-Reply-To: <9771585a101e05bb020e374f69c18d9198964fec.camel@iki.fi>
Hi Pauli,
On Wed, Jun 10, 2026, at 11:21 AM, Pauli Virtanen wrote:
> Hi,
>
> ke, 2026-06-10 kello 10:02 -0400, Luiz Augusto von Dentz kirjoitti:
>> Hi Siwei,
>>
>> On Wed, Jun 10, 2026 at 9:52 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> >
>> > Hi Luiz,
>> >
>> > On Wed, Jun 10, 2026, at 9:14 AM, Luiz Augusto von Dentz wrote:
>> > > Hi Siwei,
>> > >
>> > > On Wed, Jun 10, 2026 at 9:07 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>> > > >
>> > > > hci_abort_conn() reads hci_skb_event(hdev->sent_cmd) when a connection
>> > > > is pending, but hdev->sent_cmd can be NULL while req_status is still
>> > > > HCI_REQ_PEND
>> > >
>> > > Can it really be NULL? If it can then I don't think we are handling
>> > > HCI_REQ_PEND well and perhaps should instead just check hdev->sent_cmd
>> > > directly instead of bothering with hdev->req_status.
>> > >
>> >
>> > This is the crash stack.
>> >
>> > Title: general protection fault in hci_abort_conn
>> > Type: general protection fault
>> > Function: hci_abort_conn
>> > Signature: f64ebf7af5cee01a
>> > Time: 2026-06-01-12:48:57
>> > ---
>> > [ 1266.952280] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000007: 0000 [#1] SMP KASAN NOPTI
>> > [ 1266.954486] KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
>> > [ 1266.955864] CPU: 0 UID: 0 PID: 384 Comm: kworker/u9:1 Tainted: G W 7.0.8-g8de79710cf39 #4 PREEMPT(lazy)
>> > [ 1266.957695] Tainted: [W]=WARN
>> > [ 1266.958535] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
>> > [ 1266.960480] Workqueue: hci0 hci_rx_work
>> > [ 1266.961483] RIP: 0010:hci_abort_conn+0x179/0x2d0
>> > [ 1266.962296] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
>> > [ 1266.965380] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
>> > [ 1266.966267] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
>> > [ 1266.967770] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
>> > [ 1266.969054] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
>> > [ 1266.970874] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
>> > [ 1266.972036] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
>> > [ 1266.973213] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
>> > [ 1266.974917] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> > [ 1266.975867] CR2: 00007f51ba4e2020 CR3: 000000010fea7000 CR4: 00000000000006f0
>> > [ 1266.977409] Call Trace:
>> > [ 1266.977895] <TASK>
>> > [ 1266.978676] hci_disconnect+0xe8/0x250 net/bluetooth/hci_conn.c:197
>> > [ 1266.979781] ? lock_release+0xf8/0x370 kernel/locking/lockdep.c:5889
>> > [ 1266.980541] ? __pfx_hci_disconnect+0x10/0x10 net/bluetooth/hci_conn.c:179
>> > [ 1266.981926] ? hci_conn_hash_lookup_ba+0x29e/0x2c0 include/net/bluetooth/hci_core.h:1259
>> > [ 1266.982879] ? __crypto_memneq+0x47/0x490 lib/crypto/memneq.c:169
>> > [ 1266.983884] hci_link_key_notify_evt+0x21d/0xa50 net/bluetooth/hci_event.c:4750
>> > [ 1266.984987] ? __pfx___mutex_unlock_slowpath+0x10/0x10 kernel/locking/mutex.c:932
>> > [ 1266.985918] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
>> > [ 1266.986769] ? skb_pull_data+0x100/0x1b0 net/core/skbuff.c:2710
>> > [ 1266.987754] hci_event_packet+0x65e/0xd50
>> > [ 1266.988429] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
>> > [ 1266.989277] ? __pfx_hci_event_packet+0x10/0x10 net/bluetooth/hci_event.c:7803
>> > [ 1266.990051] ? lockdep_hardirqs_on_prepare+0x168/0x230 kernel/locking/lockdep.c:4410
>> > [ 1266.991088] ? hci_send_to_monitor+0xdd/0x510
>> > [ 1266.991977] hci_rx_work+0x3d1/0x8d0 net/bluetooth/hci_core.c:4078
>> > [ 1266.993050] ? process_scheduled_works+0xa01/0x1760 kernel/workqueue.c:3371
>> > [ 1266.994019] process_scheduled_works+0xac1/0x1760 kernel/workqueue.c:3371
>> > [ 1266.995274] ? trace_sched_exit_tp+0x35/0x130 include/trace/events/sched.h:886
>> > [ 1266.996155] ? __pfx_process_scheduled_works+0x10/0x10 kernel/workqueue.c:3361
>> > [ 1266.997132] ? do_raw_spin_lock+0x126/0x270 kernel/locking/spinlock_debug.c:116
>> > [ 1266.997955] ? assign_work+0x333/0x4d0
>> > [ 1266.998706] worker_thread+0xa22/0xf80 kernel/workqueue.c:3453
>> > [ 1266.999792] ? __kthread_parkme+0x1a5/0x200
>> > [ 1267.000591] kthread+0x38b/0x480 kernel/kthread.c:438
>> > [ 1267.001680] ? __pfx_worker_thread+0x10/0x10 kernel/workqueue.c:3398
>> > [ 1267.002499] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
>> > [ 1267.003154] ret_from_fork+0x436/0x950 arch/x86/kernel/process.c:164
>> > [ 1267.003797] ? __pfx_ret_from_fork+0x10/0x10 arch/x86/kernel/process.c:153
>> > [ 1267.004501] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
>> > [ 1267.005137] ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:258
>> > [ 1267.005855] </TASK>
>> > [ 1267.006918] Modules linked in:
>> > [ 1267.008266] ---[ end trace 0000000000000000 ]---
>> > [ 1267.010119] RIP: 0010:hci_abort_conn+0x179/0x2d0
>> > [ 1267.011723] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
>> > [ 1267.015717] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
>> > [ 1267.016793] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
>> > [ 1267.018342] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
>> > [ 1267.019857] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
>> > [ 1267.021481] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
>> > [ 1267.022973] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
>> > [ 1267.024217] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
>> > [ 1267.025954] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> >
>> > > > , leading to a NULL pointer dereference and a general
>> > > > protection fault from the hci_rx_work() receive path.
>> > > >
>> > > > Check hdev->sent_cmd before dereferencing it.
>> > > >
>> > > > Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
>> > > > Cc: stable@vger.kernel.org
>> > > > Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
>> > > > ---
>> > > > net/bluetooth/hci_conn.c | 3 ++-
>> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> > > > index 54eabaa46960..43fdacb2c9b2 100644
>> > > > --- a/net/bluetooth/hci_conn.c
>> > > > +++ b/net/bluetooth/hci_conn.c
>> > > > @@ -3188,7 +3188,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
>> > > > * hci_connect_le serializes the connection attempts so only one
>> > > > * connection can be in BT_CONNECT at time.
>> > > > */
>> > > > - if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
>> > > > + if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
>> > > > + hdev->sent_cmd) {
>> > > > switch (hci_skb_event(hdev->sent_cmd)) {
>
> The hdev->sent_cmd access here looks unsafe vs synchronization:
>
> * hdev->sent_cmd pointer is modified (only) from hci_cmd_work() which
> runs in hdev->workqueue.
>
> * No lock is held, only the ordered hdev->workqueue synchronizes
>
> * The skb pointed to is also freed from hdev->workqueue.
>
> * Even if valid pointer to it is obtained, the skb may be freed under
> us here
>
> hci_abort_conn() is called from places like smp->security_timer ->
> hci_disconnect() which run in kernel default workqueues -> potential
> UAF here.
>
> MGMT unpair_device_sync() appears to call it from hdev->req_workqueue,
> which is then the same.
>
> Taking hdev_lock doesn't help here, as it's only synchronized by the
> workqueue.
>
> The hdev->sent_cmd probably needs some new locking primitive, not sure
> hdev->lock and hdev->req_lock can be used here as not sure what locking
> contexts this is called from.
>
Thanks for your review. The patch v2 eliminate hdev->sent_cmd
access here. Please check:
https://lore.kernel.org/linux-bluetooth/20260610150704.1234558-1-oss@fourdim.xyz/T/#u
>> > > > case HCI_EV_CONN_COMPLETE:
>> > > > case HCI_EV_LE_CONN_COMPLETE:
>> > > > --
>> > > > 2.54.0
>> > > >
>> >
>> > There is an else if branch after it:
>> >
>> > if (conn->state == BT_CONNECT && hdev->sent_cmd) {
>> > 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)) {
>> > return 0;
>> > }
>> >
>> > I don't think replacing it is a wise choice as else if branch might be escaped accidentally.
>>
>> Perhaps we should change it. Maybe `hci_conn` could set a bit if it is
>> pending or just scheduled and it would probably be cleaner if
>> hci_cancel_connect_sync handles this internally.
>>
>> > e.g. status != pending can go to if branch and cancel in else if branch can never be executed.
>> >
>> > >
>> > >
>> > > --
>> > > Luiz Augusto von Dentz
>> >
>> > BTW, could you please check my explanation for another patch as well?
>> > https://lore.kernel.org/linux-bluetooth/20260603150835.3539963-1-oss@fourdim.xyz/T/#m0702d8586e0fb10775f4bd52998bf42e09b85af7
>> >
>> > Thank you.
>> >
>> > Best,
>> > Siwei
>>
>>
>
> --
> Pauli Virtanen
Best,
Siwei
^ permalink raw reply
* Re: [PATCH] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Pauli Virtanen @ 2026-06-10 15:21 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <CABBYNZKt14=Q_4RPpnZRBsnTZaOmBG3fvDHyB21+0_ndNXb5aw@mail.gmail.com>
Hi,
ke, 2026-06-10 kello 10:02 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Siwei,
>
> On Wed, Jun 10, 2026 at 9:52 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >
> > Hi Luiz,
> >
> > On Wed, Jun 10, 2026, at 9:14 AM, Luiz Augusto von Dentz wrote:
> > > Hi Siwei,
> > >
> > > On Wed, Jun 10, 2026 at 9:07 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> > > >
> > > > hci_abort_conn() reads hci_skb_event(hdev->sent_cmd) when a connection
> > > > is pending, but hdev->sent_cmd can be NULL while req_status is still
> > > > HCI_REQ_PEND
> > >
> > > Can it really be NULL? If it can then I don't think we are handling
> > > HCI_REQ_PEND well and perhaps should instead just check hdev->sent_cmd
> > > directly instead of bothering with hdev->req_status.
> > >
> >
> > This is the crash stack.
> >
> > Title: general protection fault in hci_abort_conn
> > Type: general protection fault
> > Function: hci_abort_conn
> > Signature: f64ebf7af5cee01a
> > Time: 2026-06-01-12:48:57
> > ---
> > [ 1266.952280] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000007: 0000 [#1] SMP KASAN NOPTI
> > [ 1266.954486] KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
> > [ 1266.955864] CPU: 0 UID: 0 PID: 384 Comm: kworker/u9:1 Tainted: G W 7.0.8-g8de79710cf39 #4 PREEMPT(lazy)
> > [ 1266.957695] Tainted: [W]=WARN
> > [ 1266.958535] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> > [ 1266.960480] Workqueue: hci0 hci_rx_work
> > [ 1266.961483] RIP: 0010:hci_abort_conn+0x179/0x2d0
> > [ 1266.962296] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
> > [ 1266.965380] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
> > [ 1266.966267] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
> > [ 1266.967770] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
> > [ 1266.969054] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
> > [ 1266.970874] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
> > [ 1266.972036] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
> > [ 1266.973213] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
> > [ 1266.974917] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 1266.975867] CR2: 00007f51ba4e2020 CR3: 000000010fea7000 CR4: 00000000000006f0
> > [ 1266.977409] Call Trace:
> > [ 1266.977895] <TASK>
> > [ 1266.978676] hci_disconnect+0xe8/0x250 net/bluetooth/hci_conn.c:197
> > [ 1266.979781] ? lock_release+0xf8/0x370 kernel/locking/lockdep.c:5889
> > [ 1266.980541] ? __pfx_hci_disconnect+0x10/0x10 net/bluetooth/hci_conn.c:179
> > [ 1266.981926] ? hci_conn_hash_lookup_ba+0x29e/0x2c0 include/net/bluetooth/hci_core.h:1259
> > [ 1266.982879] ? __crypto_memneq+0x47/0x490 lib/crypto/memneq.c:169
> > [ 1266.983884] hci_link_key_notify_evt+0x21d/0xa50 net/bluetooth/hci_event.c:4750
> > [ 1266.984987] ? __pfx___mutex_unlock_slowpath+0x10/0x10 kernel/locking/mutex.c:932
> > [ 1266.985918] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
> > [ 1266.986769] ? skb_pull_data+0x100/0x1b0 net/core/skbuff.c:2710
> > [ 1266.987754] hci_event_packet+0x65e/0xd50
> > [ 1266.988429] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
> > [ 1266.989277] ? __pfx_hci_event_packet+0x10/0x10 net/bluetooth/hci_event.c:7803
> > [ 1266.990051] ? lockdep_hardirqs_on_prepare+0x168/0x230 kernel/locking/lockdep.c:4410
> > [ 1266.991088] ? hci_send_to_monitor+0xdd/0x510
> > [ 1266.991977] hci_rx_work+0x3d1/0x8d0 net/bluetooth/hci_core.c:4078
> > [ 1266.993050] ? process_scheduled_works+0xa01/0x1760 kernel/workqueue.c:3371
> > [ 1266.994019] process_scheduled_works+0xac1/0x1760 kernel/workqueue.c:3371
> > [ 1266.995274] ? trace_sched_exit_tp+0x35/0x130 include/trace/events/sched.h:886
> > [ 1266.996155] ? __pfx_process_scheduled_works+0x10/0x10 kernel/workqueue.c:3361
> > [ 1266.997132] ? do_raw_spin_lock+0x126/0x270 kernel/locking/spinlock_debug.c:116
> > [ 1266.997955] ? assign_work+0x333/0x4d0
> > [ 1266.998706] worker_thread+0xa22/0xf80 kernel/workqueue.c:3453
> > [ 1266.999792] ? __kthread_parkme+0x1a5/0x200
> > [ 1267.000591] kthread+0x38b/0x480 kernel/kthread.c:438
> > [ 1267.001680] ? __pfx_worker_thread+0x10/0x10 kernel/workqueue.c:3398
> > [ 1267.002499] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
> > [ 1267.003154] ret_from_fork+0x436/0x950 arch/x86/kernel/process.c:164
> > [ 1267.003797] ? __pfx_ret_from_fork+0x10/0x10 arch/x86/kernel/process.c:153
> > [ 1267.004501] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
> > [ 1267.005137] ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:258
> > [ 1267.005855] </TASK>
> > [ 1267.006918] Modules linked in:
> > [ 1267.008266] ---[ end trace 0000000000000000 ]---
> > [ 1267.010119] RIP: 0010:hci_abort_conn+0x179/0x2d0
> > [ 1267.011723] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
> > [ 1267.015717] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
> > [ 1267.016793] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
> > [ 1267.018342] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
> > [ 1267.019857] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
> > [ 1267.021481] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
> > [ 1267.022973] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
> > [ 1267.024217] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
> > [ 1267.025954] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >
> > > > , leading to a NULL pointer dereference and a general
> > > > protection fault from the hci_rx_work() receive path.
> > > >
> > > > Check hdev->sent_cmd before dereferencing it.
> > > >
> > > > Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> > > > ---
> > > > net/bluetooth/hci_conn.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > > > index 54eabaa46960..43fdacb2c9b2 100644
> > > > --- a/net/bluetooth/hci_conn.c
> > > > +++ b/net/bluetooth/hci_conn.c
> > > > @@ -3188,7 +3188,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
> > > > * hci_connect_le serializes the connection attempts so only one
> > > > * connection can be in BT_CONNECT at time.
> > > > */
> > > > - if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
> > > > + if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
> > > > + hdev->sent_cmd) {
> > > > switch (hci_skb_event(hdev->sent_cmd)) {
The hdev->sent_cmd access here looks unsafe vs synchronization:
* hdev->sent_cmd pointer is modified (only) from hci_cmd_work() which
runs in hdev->workqueue.
* No lock is held, only the ordered hdev->workqueue synchronizes
* The skb pointed to is also freed from hdev->workqueue.
* Even if valid pointer to it is obtained, the skb may be freed under
us here
hci_abort_conn() is called from places like smp->security_timer ->
hci_disconnect() which run in kernel default workqueues -> potential
UAF here.
MGMT unpair_device_sync() appears to call it from hdev->req_workqueue,
which is then the same.
Taking hdev_lock doesn't help here, as it's only synchronized by the
workqueue.
The hdev->sent_cmd probably needs some new locking primitive, not sure
hdev->lock and hdev->req_lock can be used here as not sure what locking
contexts this is called from.
> > > > case HCI_EV_CONN_COMPLETE:
> > > > case HCI_EV_LE_CONN_COMPLETE:
> > > > --
> > > > 2.54.0
> > > >
> >
> > There is an else if branch after it:
> >
> > if (conn->state == BT_CONNECT && hdev->sent_cmd) {
> > 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)) {
> > return 0;
> > }
> >
> > I don't think replacing it is a wise choice as else if branch might be escaped accidentally.
>
> Perhaps we should change it. Maybe `hci_conn` could set a bit if it is
> pending or just scheduled and it would probably be cleaner if
> hci_cancel_connect_sync handles this internally.
>
> > e.g. status != pending can go to if branch and cancel in else if branch can never be executed.
> >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> > BTW, could you please check my explanation for another patch as well?
> > https://lore.kernel.org/linux-bluetooth/20260603150835.3539963-1-oss@fourdim.xyz/T/#m0702d8586e0fb10775f4bd52998bf42e09b85af7
> >
> > Thank you.
> >
> > Best,
> > Siwei
>
>
--
Pauli Virtanen
^ permalink raw reply
* [PATCH v2] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Siwei Zhang @ 2026-06-10 15:06 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Siwei Zhang
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,
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);
+
+ return -EBUSY;
}
int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3] Bluetooth: Fix Use-After-Free in hci_unregister_dev
From: Luiz Augusto von Dentz @ 2026-06-10 14:52 UTC (permalink / raw)
To: Jordan Walters; +Cc: linux-bluetooth, linux-kernel
In-Reply-To: <20260603075134.246832-1-jaggyaur@gmail.com>
Hi Jordan,
On Wed, Jun 3, 2026 at 4:09 AM Jordan Walters <jaggyaur@gmail.com> wrote:
>
> The hci_unregister_dev() function fails to safely cancel the cmd_timer
> and ncmd_timer before freeing the hci_dev structure. If an asynchronous
> event or timeout occurs during device teardown, the timer callbacks
> may execute after the device has been freed, leading to a KASAN
> slab-use-after-free panic.
>
> This patch adds cancel_delayed_work_sync() calls at the end of
> hci_unregister_dev() after all teardown procedures have completed.
> This ensures the timers are fully flushed before the struct is freed,
> preventing any use-after-free conditions.
>
> Signed-off-by: Jordan Walters <jaggyaur@gmail.com>
> ---
> net/bluetooth/hci_core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 28d7929dc59..3db1b3738b5 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2698,6 +2698,9 @@ void hci_unregister_dev(struct hci_dev *hdev)
> rfkill_unregister(hdev->rfkill);
> rfkill_destroy(hdev->rfkill);
> }
>
> device_del(&hdev->dev);
> +
> + cancel_delayed_work_sync(&hdev->cmd_timer);
> + cancel_delayed_work_sync(&hdev->ncmd_timer);
> +
> /* Actual cleanup is deferred until hci_release_dev(). */
> hci_dev_put(hdev);
> }
I'm not sure what's happening, but I cannot apply this change with
`git am`. Also, why did you revert to using `cancel` instead of
`disable`?
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v3] Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref
From: patchwork-bot+bluetooth @ 2026-06-10 14:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <20260609193222.192456-1-luiz.dentz@gmail.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:32:22 -0400 you wrote:
> From: Marco Elver <elver@google.com>
>
> l2cap_chan_timeout() runs asynchronously and accesses chan->conn. If
> the connection is torn down while the timer is running or pending,
> chan->conn can be freed, leading to a use-after-free when the timer
> worker attempts to lock conn->lock:
>
> [...]
Here is the summary with links:
- [v3] Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref
https://git.kernel.org/bluetooth/bluetooth-next/c/06528e2f5fc9
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: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Luiz Augusto von Dentz @ 2026-06-10 14:02 UTC (permalink / raw)
To: Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <88931814-4915-40a5-8e21-66ddbdb2a530@app.fastmail.com>
Hi Siwei,
On Wed, Jun 10, 2026 at 9:52 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>
> Hi Luiz,
>
> On Wed, Jun 10, 2026, at 9:14 AM, Luiz Augusto von Dentz wrote:
> > Hi Siwei,
> >
> > On Wed, Jun 10, 2026 at 9:07 AM Siwei Zhang <oss@fourdim.xyz> wrote:
> >>
> >> hci_abort_conn() reads hci_skb_event(hdev->sent_cmd) when a connection
> >> is pending, but hdev->sent_cmd can be NULL while req_status is still
> >> HCI_REQ_PEND
> >
> > Can it really be NULL? If it can then I don't think we are handling
> > HCI_REQ_PEND well and perhaps should instead just check hdev->sent_cmd
> > directly instead of bothering with hdev->req_status.
> >
>
> This is the crash stack.
>
> Title: general protection fault in hci_abort_conn
> Type: general protection fault
> Function: hci_abort_conn
> Signature: f64ebf7af5cee01a
> Time: 2026-06-01-12:48:57
> ---
> [ 1266.952280] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000007: 0000 [#1] SMP KASAN NOPTI
> [ 1266.954486] KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
> [ 1266.955864] CPU: 0 UID: 0 PID: 384 Comm: kworker/u9:1 Tainted: G W 7.0.8-g8de79710cf39 #4 PREEMPT(lazy)
> [ 1266.957695] Tainted: [W]=WARN
> [ 1266.958535] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [ 1266.960480] Workqueue: hci0 hci_rx_work
> [ 1266.961483] RIP: 0010:hci_abort_conn+0x179/0x2d0
> [ 1266.962296] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
> [ 1266.965380] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
> [ 1266.966267] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
> [ 1266.967770] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
> [ 1266.969054] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
> [ 1266.970874] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
> [ 1266.972036] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
> [ 1266.973213] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
> [ 1266.974917] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1266.975867] CR2: 00007f51ba4e2020 CR3: 000000010fea7000 CR4: 00000000000006f0
> [ 1266.977409] Call Trace:
> [ 1266.977895] <TASK>
> [ 1266.978676] hci_disconnect+0xe8/0x250 net/bluetooth/hci_conn.c:197
> [ 1266.979781] ? lock_release+0xf8/0x370 kernel/locking/lockdep.c:5889
> [ 1266.980541] ? __pfx_hci_disconnect+0x10/0x10 net/bluetooth/hci_conn.c:179
> [ 1266.981926] ? hci_conn_hash_lookup_ba+0x29e/0x2c0 include/net/bluetooth/hci_core.h:1259
> [ 1266.982879] ? __crypto_memneq+0x47/0x490 lib/crypto/memneq.c:169
> [ 1266.983884] hci_link_key_notify_evt+0x21d/0xa50 net/bluetooth/hci_event.c:4750
> [ 1266.984987] ? __pfx___mutex_unlock_slowpath+0x10/0x10 kernel/locking/mutex.c:932
> [ 1266.985918] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
> [ 1266.986769] ? skb_pull_data+0x100/0x1b0 net/core/skbuff.c:2710
> [ 1266.987754] hci_event_packet+0x65e/0xd50
> [ 1266.988429] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
> [ 1266.989277] ? __pfx_hci_event_packet+0x10/0x10 net/bluetooth/hci_event.c:7803
> [ 1266.990051] ? lockdep_hardirqs_on_prepare+0x168/0x230 kernel/locking/lockdep.c:4410
> [ 1266.991088] ? hci_send_to_monitor+0xdd/0x510
> [ 1266.991977] hci_rx_work+0x3d1/0x8d0 net/bluetooth/hci_core.c:4078
> [ 1266.993050] ? process_scheduled_works+0xa01/0x1760 kernel/workqueue.c:3371
> [ 1266.994019] process_scheduled_works+0xac1/0x1760 kernel/workqueue.c:3371
> [ 1266.995274] ? trace_sched_exit_tp+0x35/0x130 include/trace/events/sched.h:886
> [ 1266.996155] ? __pfx_process_scheduled_works+0x10/0x10 kernel/workqueue.c:3361
> [ 1266.997132] ? do_raw_spin_lock+0x126/0x270 kernel/locking/spinlock_debug.c:116
> [ 1266.997955] ? assign_work+0x333/0x4d0
> [ 1266.998706] worker_thread+0xa22/0xf80 kernel/workqueue.c:3453
> [ 1266.999792] ? __kthread_parkme+0x1a5/0x200
> [ 1267.000591] kthread+0x38b/0x480 kernel/kthread.c:438
> [ 1267.001680] ? __pfx_worker_thread+0x10/0x10 kernel/workqueue.c:3398
> [ 1267.002499] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
> [ 1267.003154] ret_from_fork+0x436/0x950 arch/x86/kernel/process.c:164
> [ 1267.003797] ? __pfx_ret_from_fork+0x10/0x10 arch/x86/kernel/process.c:153
> [ 1267.004501] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
> [ 1267.005137] ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:258
> [ 1267.005855] </TASK>
> [ 1267.006918] Modules linked in:
> [ 1267.008266] ---[ end trace 0000000000000000 ]---
> [ 1267.010119] RIP: 0010:hci_abort_conn+0x179/0x2d0
> [ 1267.011723] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
> [ 1267.015717] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
> [ 1267.016793] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
> [ 1267.018342] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
> [ 1267.019857] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
> [ 1267.021481] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
> [ 1267.022973] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
> [ 1267.024217] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
> [ 1267.025954] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>
> >> , leading to a NULL pointer dereference and a general
> >> protection fault from the hci_rx_work() receive path.
> >>
> >> Check hdev->sent_cmd before dereferencing it.
> >>
> >> Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> >> ---
> >> net/bluetooth/hci_conn.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >> index 54eabaa46960..43fdacb2c9b2 100644
> >> --- a/net/bluetooth/hci_conn.c
> >> +++ b/net/bluetooth/hci_conn.c
> >> @@ -3188,7 +3188,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
> >> * hci_connect_le serializes the connection attempts so only one
> >> * connection can be in BT_CONNECT at time.
> >> */
> >> - if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
> >> + if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
> >> + hdev->sent_cmd) {
> >> switch (hci_skb_event(hdev->sent_cmd)) {
> >> case HCI_EV_CONN_COMPLETE:
> >> case HCI_EV_LE_CONN_COMPLETE:
> >> --
> >> 2.54.0
> >>
>
> There is an else if branch after it:
>
> if (conn->state == BT_CONNECT && hdev->sent_cmd) {
> 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)) {
> return 0;
> }
>
> I don't think replacing it is a wise choice as else if branch might be escaped accidentally.
Perhaps we should change it. Maybe `hci_conn` could set a bit if it is
pending or just scheduled and it would probably be cleaner if
hci_cancel_connect_sync handles this internally.
> e.g. status != pending can go to if branch and cancel in else if branch can never be executed.
>
> >
> >
> > --
> > Luiz Augusto von Dentz
>
> BTW, could you please check my explanation for another patch as well?
> https://lore.kernel.org/linux-bluetooth/20260603150835.3539963-1-oss@fourdim.xyz/T/#m0702d8586e0fb10775f4bd52998bf42e09b85af7
>
> Thank you.
>
> Best,
> Siwei
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Siwei Zhang @ 2026-06-10 13:52 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <CABBYNZKCciLjShDCi0NR53WA9KvQjMo_sMyEwtvWFCsJ0MSZDw@mail.gmail.com>
Hi Luiz,
On Wed, Jun 10, 2026, at 9:14 AM, Luiz Augusto von Dentz wrote:
> Hi Siwei,
>
> On Wed, Jun 10, 2026 at 9:07 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>>
>> hci_abort_conn() reads hci_skb_event(hdev->sent_cmd) when a connection
>> is pending, but hdev->sent_cmd can be NULL while req_status is still
>> HCI_REQ_PEND
>
> Can it really be NULL? If it can then I don't think we are handling
> HCI_REQ_PEND well and perhaps should instead just check hdev->sent_cmd
> directly instead of bothering with hdev->req_status.
>
This is the crash stack.
Title: general protection fault in hci_abort_conn
Type: general protection fault
Function: hci_abort_conn
Signature: f64ebf7af5cee01a
Time: 2026-06-01-12:48:57
---
[ 1266.952280] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000007: 0000 [#1] SMP KASAN NOPTI
[ 1266.954486] KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
[ 1266.955864] CPU: 0 UID: 0 PID: 384 Comm: kworker/u9:1 Tainted: G W 7.0.8-g8de79710cf39 #4 PREEMPT(lazy)
[ 1266.957695] Tainted: [W]=WARN
[ 1266.958535] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 1266.960480] Workqueue: hci0 hci_rx_work
[ 1266.961483] RIP: 0010:hci_abort_conn+0x179/0x2d0
[ 1266.962296] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
[ 1266.965380] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
[ 1266.966267] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
[ 1266.967770] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
[ 1266.969054] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
[ 1266.970874] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
[ 1266.972036] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
[ 1266.973213] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
[ 1266.974917] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1266.975867] CR2: 00007f51ba4e2020 CR3: 000000010fea7000 CR4: 00000000000006f0
[ 1266.977409] Call Trace:
[ 1266.977895] <TASK>
[ 1266.978676] hci_disconnect+0xe8/0x250 net/bluetooth/hci_conn.c:197
[ 1266.979781] ? lock_release+0xf8/0x370 kernel/locking/lockdep.c:5889
[ 1266.980541] ? __pfx_hci_disconnect+0x10/0x10 net/bluetooth/hci_conn.c:179
[ 1266.981926] ? hci_conn_hash_lookup_ba+0x29e/0x2c0 include/net/bluetooth/hci_core.h:1259
[ 1266.982879] ? __crypto_memneq+0x47/0x490 lib/crypto/memneq.c:169
[ 1266.983884] hci_link_key_notify_evt+0x21d/0xa50 net/bluetooth/hci_event.c:4750
[ 1266.984987] ? __pfx___mutex_unlock_slowpath+0x10/0x10 kernel/locking/mutex.c:932
[ 1266.985918] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
[ 1266.986769] ? skb_pull_data+0x100/0x1b0 net/core/skbuff.c:2710
[ 1266.987754] hci_event_packet+0x65e/0xd50
[ 1266.988429] ? __pfx_hci_link_key_notify_evt+0x10/0x10 net/bluetooth/hci_event.c:4730
[ 1266.989277] ? __pfx_hci_event_packet+0x10/0x10 net/bluetooth/hci_event.c:7803
[ 1266.990051] ? lockdep_hardirqs_on_prepare+0x168/0x230 kernel/locking/lockdep.c:4410
[ 1266.991088] ? hci_send_to_monitor+0xdd/0x510
[ 1266.991977] hci_rx_work+0x3d1/0x8d0 net/bluetooth/hci_core.c:4078
[ 1266.993050] ? process_scheduled_works+0xa01/0x1760 kernel/workqueue.c:3371
[ 1266.994019] process_scheduled_works+0xac1/0x1760 kernel/workqueue.c:3371
[ 1266.995274] ? trace_sched_exit_tp+0x35/0x130 include/trace/events/sched.h:886
[ 1266.996155] ? __pfx_process_scheduled_works+0x10/0x10 kernel/workqueue.c:3361
[ 1266.997132] ? do_raw_spin_lock+0x126/0x270 kernel/locking/spinlock_debug.c:116
[ 1266.997955] ? assign_work+0x333/0x4d0
[ 1266.998706] worker_thread+0xa22/0xf80 kernel/workqueue.c:3453
[ 1266.999792] ? __kthread_parkme+0x1a5/0x200
[ 1267.000591] kthread+0x38b/0x480 kernel/kthread.c:438
[ 1267.001680] ? __pfx_worker_thread+0x10/0x10 kernel/workqueue.c:3398
[ 1267.002499] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
[ 1267.003154] ret_from_fork+0x436/0x950 arch/x86/kernel/process.c:164
[ 1267.003797] ? __pfx_ret_from_fork+0x10/0x10 arch/x86/kernel/process.c:153
[ 1267.004501] ? __pfx_kthread+0x10/0x10 kernel/kthread.c:381
[ 1267.005137] ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:258
[ 1267.005855] </TASK>
[ 1267.006918] Modules linked in:
[ 1267.008266] ---[ end trace 0000000000000000 ]---
[ 1267.010119] RIP: 0010:hci_abort_conn+0x179/0x2d0
[ 1267.011723] Code: 8d be 48 0e 00 00 4c 89 f8 48 c1 e8 03 42 80 3c 28 00 74 08 4c 89 ff e8 05 a8 9b fc 4d 8b 3f 49 83 c7 3b 4c 89 f8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 29 01 00 00 45 0f b6 3f 4c 89 ff 48 c7
[ 1267.015717] RSP: 0018:ffffc9000a277760 EFLAGS: 00010212
[ 1267.016793] RAX: 0000000000000007 RBX: ffff888114d4c000 RCX: 0000000000000000
[ 1267.018342] RDX: ffff8881110f3600 RSI: 0000000000000001 RDI: 0000000000000001
[ 1267.019857] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
[ 1267.021481] R10: 0000000000000000 R11: ffffffff85464c1b R12: 0000000000000005
[ 1267.022973] R13: dffffc0000000000 R14: ffff88810f810000 R15: 000000000000003b
[ 1267.024217] FS: 0000000000000000(0000) GS:ffff888190689000(0000) knlGS:0000000000000000
[ 1267.025954] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> , leading to a NULL pointer dereference and a general
>> protection fault from the hci_rx_work() receive path.
>>
>> Check hdev->sent_cmd before dereferencing it.
>>
>> Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
>> ---
>> net/bluetooth/hci_conn.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 54eabaa46960..43fdacb2c9b2 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -3188,7 +3188,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
>> * hci_connect_le serializes the connection attempts so only one
>> * connection can be in BT_CONNECT at time.
>> */
>> - if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
>> + if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
>> + hdev->sent_cmd) {
>> switch (hci_skb_event(hdev->sent_cmd)) {
>> case HCI_EV_CONN_COMPLETE:
>> case HCI_EV_LE_CONN_COMPLETE:
>> --
>> 2.54.0
>>
There is an else if branch after it:
if (conn->state == BT_CONNECT && hdev->sent_cmd) {
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)) {
return 0;
}
I don't think replacing it is a wise choice as else if branch might be escaped accidentally.
e.g. status != pending can go to if branch and cancel in else if branch can never be executed.
>
>
> --
> Luiz Augusto von Dentz
BTW, could you please check my explanation for another patch as well?
https://lore.kernel.org/linux-bluetooth/20260603150835.3539963-1-oss@fourdim.xyz/T/#m0702d8586e0fb10775f4bd52998bf42e09b85af7
Thank you.
Best,
Siwei
^ permalink raw reply
* [bluez/bluez]
From: BluezTestBot @ 2026-06-10 13:49 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/heads/1092705
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] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Luiz Augusto von Dentz @ 2026-06-10 13:14 UTC (permalink / raw)
To: Siwei Zhang; +Cc: linux-bluetooth
In-Reply-To: <20260610130648.1091326-1-oss@fourdim.xyz>
Hi Siwei,
On Wed, Jun 10, 2026 at 9:07 AM Siwei Zhang <oss@fourdim.xyz> wrote:
>
> hci_abort_conn() reads hci_skb_event(hdev->sent_cmd) when a connection
> is pending, but hdev->sent_cmd can be NULL while req_status is still
> HCI_REQ_PEND
Can it really be NULL? If it can then I don't think we are handling
HCI_REQ_PEND well and perhaps should instead just check hdev->sent_cmd
directly instead of bothering with hdev->req_status.
, leading to a NULL pointer dereference and a general
> protection fault from the hci_rx_work() receive path.
>
> Check hdev->sent_cmd before dereferencing it.
>
> Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
> Cc: stable@vger.kernel.org
> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> ---
> net/bluetooth/hci_conn.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 54eabaa46960..43fdacb2c9b2 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -3188,7 +3188,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
> * hci_connect_le serializes the connection attempts so only one
> * connection can be in BT_CONNECT at time.
> */
> - if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
> + if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
> + hdev->sent_cmd) {
> switch (hci_skb_event(hdev->sent_cmd)) {
> case HCI_EV_CONN_COMPLETE:
> case HCI_EV_LE_CONN_COMPLETE:
> --
> 2.54.0
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH BlueZ] shared/bap: Transition ASE to QoS Configured on CIS loss
From: Luiz Augusto von Dentz @ 2026-06-10 13:10 UTC (permalink / raw)
To: Simon Mikuda; +Cc: Pauli Virtanen, linux-bluetooth
In-Reply-To: <ac997f34-8c43-4935-bc78-90fa5eef714b@streamunlimited.com>
Hi Simon,
On Wed, Jun 10, 2026 at 4:03 AM Simon Mikuda
<simon.mikuda@streamunlimited.com> wrote:
>
> this check is for ep (struct bt_bap_endpoint) that is used only for
> unicast. broadcasts use different state in struct bt_bap_stream, so it
> should be OK
>
> On 6/10/26 00:15, Pauli Virtanen wrote:
> > ti, 2026-06-09 kello 23:11 +0200, Simon Mikuda kirjoitti:
> >> stream_io_disconnected() only handled the Releasing state, leaving
> >> Enabling, Streaming and Disabling ASEs stuck when the CIS was lost
> >> unexpectedly. The ASE shall autonomously move to QoS Configured on loss
> >> of the CIS and notify the peer; add that transition.
> >>
> >> Fixes PTS test BAP/USR/SCC/BV-167-C
> >> ---
> >> src/shared/bap.c | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/src/shared/bap.c b/src/shared/bap.c
> >> index deb85b264..350ed53d9 100644
> >> --- a/src/shared/bap.c
> >> +++ b/src/shared/bap.c
> >> @@ -6779,6 +6779,14 @@ static bool stream_io_disconnected(struct io *io, void *user_data)
> >> if (stream->ep->state == BT_ASCS_ASE_STATE_RELEASING)
> >> stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> >>
> >> + /* On loss of the CIS the ASE shall autonomously transition to QoS
> >> + * Configured and notify the peer.
> >> + */
> >> + if (stream->ep->state == BT_ASCS_ASE_STATE_STREAMING ||
> >> + stream->ep->state == BT_ASCS_ASE_STATE_ENABLING ||
> >> + stream->ep->state == BT_ASCS_ASE_STATE_DISABLING)
> >> + stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
> >> +
I think converting this to a switch statement might be better, as it
would allow us to handle all states, including releasing, within it.
It would be great to have test-bap cover these types of tests as well.
> >> bt_bap_stream_set_io(stream, -1);
> >> return false;
> >> }
> > iirc it may be also broadcast source here, does it do the right thing?
> >
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH] Bluetooth: hci_conn: Fix null ptr deref in hci_abort_conn()
From: Siwei Zhang @ 2026-06-10 13:06 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Siwei Zhang
hci_abort_conn() reads hci_skb_event(hdev->sent_cmd) when a connection
is 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.
Check hdev->sent_cmd before dereferencing it.
Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
Cc: stable@vger.kernel.org
Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
---
net/bluetooth/hci_conn.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 54eabaa46960..43fdacb2c9b2 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -3188,7 +3188,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
* hci_connect_le serializes the connection attempts so only one
* connection can be in BT_CONNECT at time.
*/
- if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
+ if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
+ hdev->sent_cmd) {
switch (hci_skb_event(hdev->sent_cmd)) {
case HCI_EV_CONN_COMPLETE:
case HCI_EV_LE_CONN_COMPLETE:
--
2.54.0
^ permalink raw reply related
* Re: [PATCH BlueZ] shared/bap: Don't link ucast streams before CIS IDs are assigned
From: Luiz Augusto von Dentz @ 2026-06-10 13:06 UTC (permalink / raw)
To: Simon Mikuda; +Cc: Pauli Virtanen, linux-bluetooth
In-Reply-To: <fa0febe1-5866-4a3f-9f86-ae70e9d4b22d@streamunlimited.com>
Hi Simon,
On Wed, Jun 10, 2026 at 3:51 AM Simon Mikuda
<simon.mikuda@streamunlimited.com> wrote:
>
> I looked into it and 0 is valid ID (probably not used, but still valid).
>
> There is a ugly part in bluez though, when stream is allocated cig and
> cis in qos struct are 0 not 0xff:
>
> stream = new0(struct bt_bap_stream, 1); /* qos all-zero: cig/cis = 0 */
>
> and also in tests there are some struct comparisons that also use
> cig/cis == 0. Some parts check for 0xff (BT_ISO_QOS_CIG_UNSET,
> BT_ISO_QOS_CIS_UNSET)
We should probably initialize it with unset and then check if CIS/CIG
have been set upon linking.
> Maybe more clean fix would be to hook this up to state, so that when
> CIG/CIS is not at least configured it is rejected e.g.:
>
> if (stream->ep->state < BT_ASCS_ASE_STATE_QOS || link->ep->state <
> BT_ASCS_ASE_STATE_QOS)
> return -EINVAL;
>
> Also probably i should cleanup all structs (code and tests), so that cig
> and cis are initialized properly with 0xff. And then provide the fix?
>
> On 6/10/26 00:03, Pauli Virtanen wrote:
> > ti, 2026-06-09 kello 23:11 +0200, Simon Mikuda kirjoitti:
> >> bap_ucast_io_link pairs streams whose CIG/CIS IDs match, but the IDs
> >> are unset in Codec Configured state, so a Sink and Source bound for
> >> different CISes get linked. The stray link later propagates a
> >> disconnect to the wrong ASE and breaks Receiver Start Ready.
> >>
> >> Skip linking until QoS Configured assigns the IDs.
> >>
> >> Fixes PTS test BAP/USR/STR/BV-362-C
> >> ---
> >> src/shared/bap.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/src/shared/bap.c b/src/shared/bap.c
> >> index deb85b264..98537de60 100644
> >> --- a/src/shared/bap.c
> >> +++ b/src/shared/bap.c
> >> @@ -2679,6 +2679,12 @@ static int bap_ucast_io_link(struct bt_bap_stream *stream,
> >> stream->ep->dir == link->ep->dir)
> >> return -EINVAL;
> >>
> >> + /* Don't link until QoS Configured assigns the CIS IDs; while unset
> >> + * the check above would pair unrelated streams.
> >> + */
> >> + if (!stream->qos.ucast.cis_id || !link->qos.ucast.cis_id)
> >> + return -EINVAL;
> > Zero is valid CIS ID?
> >
> >> +
> >> if (stream->client && !(stream->locked && link->locked))
> >> return -EINVAL;
> >>
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH] 6lowpan: fix NHC entry use-after-free on error path
From: Alexander Aring @ 2026-06-10 13:05 UTC (permalink / raw)
To: Yizhou Zhao
Cc: linux-bluetooth, Alexander Aring, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-wpan, netdev,
linux-kernel, Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu,
stable
In-Reply-To: <20260609080054.4541-1-zhaoyz24@mails.tsinghua.edu.cn>
Hi,
On Tue, Jun 9, 2026 at 4:03 AM Yizhou Zhao
<zhaoyz24@mails.tsinghua.edu.cn> wrote:
>
> lowpan_nhc_do_uncompression() looks up an NHC descriptor while holding
> lowpan_nhc_lock. If the descriptor has no uncompress callback, the error
> path drops the lock before printing nhc->name.
>
> lowpan_nhc_del() removes descriptors under the same lock and then relies
> on synchronize_net() before the owning module can be unloaded. That only
> waits for net RX RCU readers. lowpan_header_decompress() is also exported
> and can be reached from callers that are not necessarily covered by the net
> core RX critical section, for example the Bluetooth 6LoWPAN L2CAP receive
> path.
>
> This leaves a race where one task drops lowpan_nhc_lock in the error path,
> another task unregisters and frees the matching descriptor after
> synchronize_net() returns, and the first task then dereferences nhc->name
> for the warning.
>
> With the post-unlock window widened, KASAN reports:
>
> BUG: KASAN: slab-use-after-free in lowpan_nhc_do_uncompression+0x1f4/0x220
> Read of size 8
> lowpan_nhc_do_uncompression
> lowpan_header_decompress
>
> Fix this by printing the warning before dropping lowpan_nhc_lock, so the
> descriptor name is read while unregister is still excluded. The malformed
> packet is still rejected with -ENOTSUPP.
>
> Fixes: 92aa7c65d295 ("6lowpan: add generic nhc layer interface")
> Cc: stable@vger.kernel.org
> Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
> Reported-by: Ao Wang <wangao@seu.edu.cn>
> Reported-by: Xuewei Feng <fengxw06@126.com>
> Reported-by: Qi Li <qli01@tsinghua.edu.cn>
> Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
> Assisted-by: GLM:GLM-5.1
> Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
looks good. Thanks.
Acked-by: Alexander Aring <aahringo@redhat.com>
- Alex
^ 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