Re: [PATCH] Bluetooth: L2CAP: clear the SDU before the receive callback

[Luiz Augusto von Dentz]

Hi Piyus,

On Thu, Nov 12, 2026 at 12:38 PM Piyus Kedia <piyus@xxxxxxxxxxx> wrote:
>
> l2cap_ecred_data_rcv() completes a reassembled SDU by calling
> l2cap_ecred_recv() with chan->sdu still set, and only clears the
> reassembly state after it returns:
>
>         if (chan->sdu->len == chan->sdu_len) {
>                 err = l2cap_ecred_recv(chan, chan->sdu);
>                 if (!err) {
>                         chan->sdu = NULL;
>                         ...
>
> l2cap_ecred_recv() hands the SDU to the channel's receive callback,
> which takes ownership of it on success, and then updates the credits:
>
>         err = chan->ops->recv(chan, skb);
>         ...
>         l2cap_chan_le_send_credits(chan);
>
> l2cap_chan_le_send_credits() calls l2cap_le_rx_credits(), which reads
>
>         size_t sdu_len = chan->sdu ? chan->sdu->len : 0;
>
> so between the callback consuming the skb and the assignment above,
> chan->sdu is a dangling pointer that is dereferenced.
>
> For the 6LoWPAN channel the callback frees the skb outright -
> chan_recv_cb() -> recv_pkt() ends in consume_skb(skb) and returns

Looks like this comment doesn't hold:

https://sashiko.dev/#/patchset/20261112173812.18696-2-piyus%40iiitd.ac.in

The 6LoWPAN does actually free on error as well.

> NET_RX_SUCCESS - so the read is a straightforward use-after-free that
> KASAN reports:
>
>   BUG: KASAN: slab-use-after-free in l2cap_le_rx_credits
>        net/bluetooth/l2cap_core.c:576 [inline]
>   BUG: KASAN: slab-use-after-free in l2cap_chan_le_send_credits+0xa8/0x3e0
>        net/bluetooth/l2cap_core.c:6811
>   Read of size 4 at addr ffff88800e134070 by task kworker/u21:2/11352
>   Workqueue: hci0 hci_rx_work
>    l2cap_le_rx_credits net/bluetooth/l2cap_core.c:576 [inline]
>    l2cap_chan_le_send_credits+0xa8/0x3e0 net/bluetooth/l2cap_core.c:6811
>    l2cap_ecred_recv+0x63/0x120 net/bluetooth/l2cap_core.c:6873
>    l2cap_ecred_data_rcv net/bluetooth/l2cap_core.c:6976 [inline]
>    l2cap_data_channel net/bluetooth/l2cap_core.c:7028 [inline]
>    l2cap_recv_frame+0x6c3c/0x9650 net/bluetooth/l2cap_core.c:7162
>    l2cap_recv_acldata+0x6fd/0xe60 net/bluetooth/l2cap_core.c:7902
>    hci_acldata_packet net/bluetooth/hci_core.c:3865 [inline]
>    hci_rx_work+0x3c0/0xbe0 net/bluetooth/hci_core.c:4092
>   Freed by task 11352:
>    recv_pkt net/bluetooth/6lowpan.c
>    chan_recv_cb+0x88/0x150 net/bluetooth/6lowpan.c:367
>    l2cap_ecred_recv+0x61/0x160 net/bluetooth/l2cap_core.c:6864
>   The buggy address belongs to the object at ffff88800e134000
>    which belongs to the cache skbuff_head_cache of size 240
>   The buggy address is located 112 bytes inside of
>    freed 240-byte region
>
> Offset 112 is sk_buff.len.
>
> Only a multi-frame SDU is affected: the single-frame path already calls
> l2cap_ecred_recv() with chan->sdu NULL, so l2cap_le_rx_credits() reads a
> zero sdu_len there.
>
> l2cap_reassemble_sdu() completes a segmented SDU the same way and has the
> same defect.  On an ERTM or streaming channel l2cap_sock_recv_cb() calls
> l2cap_publish_rx_avail(), which reaches l2cap_chan_le_send_credits()
> through l2cap_chan_rx_avail(), and that function evaluates
>
>         u16 return_credits = l2cap_le_rx_credits(chan);
>
> in its initializer - before the mode test that makes the rest of it a
> no-op for a non-LE channel.  So chan->sdu is dereferenced on that path
> too, and the result is discarded.  What makes it a use-after-free rather
> than a stale read is a consumer that frees the skb from another thread:
> BNEP's session kthread takes it off the socket with a lockless
> skb_dequeue() (net/bluetooth/bnep/core.c:530, freed at :533) and never
> takes the socket lock, so lock_sock() is no defence.
>
> Fix both sites.
>
> Clear the reassembly state before handing the SDU to the callback, which
> makes the fragmented path behave like the single-frame one, and free the
> SDU on the spot if the callback refused it - the error path's
> kfree_skb(chan->sdu) used to do that, and the callbacks leave the skb to
> the caller when they fail.
>
> Clearing it first is also the arithmetically correct input, not merely a
> safe one.  l2cap_le_rx_credits() subtracts sdu_len from chan->rx_avail
> because a partially reassembled SDU will claim that much receive buffer
> once it completes:
>
>         if (chan->rx_avail <= sdu_len)
>                 return 0;
>
>         return DIV_ROUND_UP(chan->rx_avail - sdu_len, chan->mps);
>
> A completed SDU is no longer waiting for that space.  If the callback
> queued it, l2cap_sock_recv_cb() has already called
> l2cap_publish_rx_avail(), which recomputes rx_avail from
> sk_rcvbuf - sk_rmem_alloc with that skb included in sk_rmem_alloc; if the
> callback consumed it, as the 6LoWPAN one does, it occupies nothing at
> all.  Either way the reassembly buffer is empty, so zero is the right
> sdu_len, and a NULL chan->sdu is how l2cap_le_rx_credits() expresses
> that - the same value l2cap_le_flowctl_init() depends on when it seeds
> chan->rx_credits on an idle channel.  Carrying the old length into the
> calculation instead would subtract the SDU a second time and under-grant
> credits on every delivery, and once rx_avail fell to the size of one SDU
> the channel would be granted none at all.
>
> Per Documentation/process/generated-content.rst: the use-after-free was
> found by an AI-driven fuzzing harness built on syzkaller, developed
> jointly by Kapil Vaswani and myself, which drove a virtual HCI
> controller and a 6LoWPAN channel under KASAN on x86_64.  The root-cause
> analysis, this changelog and the patch were written with the assistance
> of a large language model over an extended session, and reviewed by
> hand; the before/after testing below the --- was run by us.
>
> Signed-off-by: Kapil Vaswani <kapil@xxxxxxxxxxxxxxx>
> Signed-off-by: Piyus Kedia <piyus@xxxxxxxxxxx>
> ---
>
> Tested on net/main itself, before and after.  The kernel was built from
> a2fd526d6442 with x86_64 defconfig plus KASAN and the Bluetooth, 6LoWPAN,
> vhci and configfs options the test needs, gcc, no instrumentation, and
> booted by hand under qemu against the same unmodified rootfs each time
> (-snapshot, so the image is never written).  The only difference between the
> two kernels is this patch:
>
>   unpatched  7.4.0-rc1-ga2fd526d6442   KASAN on every run (4 runs today)
>   patched    7.4.0-rc1-g63fe746a609a   no report (2 runs)
>
> WHICH SITE THAT COVERS, because the patch changes two.  The reproducer
> drives the ECRED path, so l2cap_ecred_data_rcv() is the hunk those runs
> exercise.  For the l2cap_reassemble_sdu() hunk, the same patched commit was
> rebuilt with CONFIG_BT_BNEP=y and driven with a segmented SDU on an ERTM
> channel with a BNEP session attached, under two kprobes:
>
>   l2cap_reassemble_sdu   14 hits
>   l2cap_sock_recv_cb      6 hits
>   interleaved rr-C rr-C: two entries into the reassembly function per
>   callback, which is a START frame installing chan->sdu followed by an END
>   frame completing it - the branch this patch changes - and not the
>   unsegmented path, which would call back on every entry
>
> with no KASAN report.  So the changed line runs and reassembly still
> completes.  What that run does NOT do is reproduce the use-after-free on
> that path: it needs the freeing thread to win, and the harness that found
> this bug did win it with BNEP's lockless skb_dequeue(), where we did not.
> The argument for why that site is affected is the ordering one above, from
> the source.
>
> The clean runs are not setup failures: on both kernels the reproducer
> reported the same L2CAP LE channel and the same frames -
>
>   SUMMARY mtu=1280 mps=1017 credits=2 scid=64 chan_gone=0
>
> - and exited 0.  Only the report differs.  On the unpatched kernel the
> console shows the program's own marker immediately before the trace:
>
>   [   45.878309] LOWRUN sdu
>   [   46.063560] BUG: KASAN: slab-use-after-free in
>                  l2cap_chan_le_send_credits+0x86/0x1d0
>   [   46.063711] Read of size 4 at addr ffff88800c9a82f0 by task
>                  kworker/u21:0/353
>   [   46.063838] Workqueue: hci0 hci_rx_work
>
> Offset 112 of the freed 240-byte skbuff_head_cache object is sk_buff.len.
> On the patched kernel the same marker is printed and nothing follows it.
>
> The same reproducer also faults on an unmodified 7.3.1 kernel, and the
> harness hit the signature 19 times over eight hours on a third kernel.
>
> No Fixes: tag: our clone is shallow, only 27 commits of l2cap_core.c history
> are reachable and git blame lands on the shallow boundary, so we cannot name
> the introducing commit.  The code is byte-identical in net/main at
> a2fd526d6442, in mainline at 79eb6679ef6a and in the 7.3.1 tree this was
> found on, and is present in every v7.3-rc tag available to us.  Please add
> the tag if you can resolve it against full history.
>
> Reproducer: not included here.  This bug was found with AI assistance, which
> Documentation/process/security-bugs.rst says to treat as public; the same
> paragraph says not to post the reproducer publicly.  It is one C program
> needing no Bluetooth hardware, it is tested, and we will send it on request.
>
> What bounds the scope: the channel's receive callback has to be one that
> frees the skb, which is the 6LoWPAN one, and the SDU has to arrive in more
> than one frame.  An administrator enables 6LoWPAN through debugfs; after
> that bt_6lowpan_listen() listens on L2CAP_PSM_IPSP and a connected LE peer
> reaches this with an ordinary fragmented SDU, needing no local privilege.
>
>  net/bluetooth/l2cap_core.c | 53 ++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7dcb0586a10b..fb965e4d75f2 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -6186,6 +6186,7 @@ static void append_skb_frag(struct sk_buff *skb, struct sk_buff *new_frag,
>  static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
>                                 struct l2cap_ctrl *control)
>  {
> +       struct sk_buff *sdu;
>         int err = -EINVAL;
>
>         switch (control->sar) {
> @@ -6246,15 +6247,25 @@ static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
>                 if (chan->sdu->len != chan->sdu_len)
>                         break;
>
> -               err = chan->ops->recv(chan, chan->sdu);
> +               /* Reassembly complete.  Hand the SDU over before the
> +                * callback, for the reason given in l2cap_ecred_data_rcv():
> +                * the callback takes ownership and may free it, while
> +                * l2cap_chan_rx_avail() -> l2cap_chan_le_send_credits() reads
> +                * chan->sdu->len on the way out - it does so before the mode
> +                * test that makes the rest of that function a no-op here.
> +                * Return directly, since the state this leaves is what the
> +                * trailing error block would otherwise set again.
> +                */
> +               sdu = chan->sdu;
> +               chan->sdu = NULL;
> +               chan->sdu_last_frag = NULL;
> +               chan->sdu_len = 0;
>
> -               if (!err) {
> -                       /* Reassembly complete */
> -                       chan->sdu = NULL;
> -                       chan->sdu_last_frag = NULL;
> -                       chan->sdu_len = 0;
> -               }
> -               break;
> +               err = chan->ops->recv(chan, sdu);
> +               if (err)
> +                       kfree_skb(sdu);
> +
> +               return err;
>         }
>
>         if (err) {
> @@ -7245,12 +7256,26 @@ static int l2cap_ecred_data_rcv(struct l2cap_chan *chan, struct sk_buff *skb)
>         skb = NULL;
>
>         if (chan->sdu->len == chan->sdu_len) {
> -               err = l2cap_ecred_recv(chan, chan->sdu);
> -               if (!err) {
> -                       chan->sdu = NULL;
> -                       chan->sdu_last_frag = NULL;
> -                       chan->sdu_len = 0;
> -               }
> +               struct sk_buff *sdu = chan->sdu;
> +
> +               /* The receive callback takes ownership of a reassembled SDU and
> +                * may free it, while l2cap_ecred_recv() goes on to update the
> +                * credits - which reads chan->sdu->len.  Drop the reassembly
> +                * state before handing the SDU over, and release it here if the
> +                * callback refused it, so that it is still freed exactly once.
> +                * The reassembly state is already clear at that point, so
> +                * return instead of falling into the failed: block, whose
> +                * kfree_skb(chan->sdu) would have nothing left to free.
> +                */
> +               chan->sdu = NULL;
> +               chan->sdu_last_frag = NULL;
> +               chan->sdu_len = 0;
> +
> +               err = l2cap_ecred_recv(chan, sdu);
> +               if (err)
> +                       kfree_skb(sdu);
> +
> +               return 0;
>         }
>
>  failed:
>
> base-commit: a2fd526d644233d6a892c28173f07c61e21e4c93
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz