Re: [PATCH] Bluetooth: L2CAP: handle zero txwin_size in ERTM RFC option

[Luiz Augusto von Dentz]

Hi Michael,

On Fri, Apr 17, 2026 at 6:16 PM Michael Bommarito
<michael.bommarito@xxxxxxxxx> wrote:
>
> Bluetooth L2CAP ERTM configuration (RFC option, type 0x04) carries an
> unsigned 8-bit txwin_size field.  Core Spec v5.3, Vol 3 Part A, section
> 5.4 specifies a valid range of 1..63 (or 1..0x3fff under the Extended
> Window Size extension).  A peer-supplied value of zero is out of spec
> but the current l2cap_parse_conf_req() path stores it into
> chan->remote_tx_win unchanged whenever CONF_EWS_RECV is not set.
>
> The zero then reaches l2cap_seq_list_init(size = 0), which computes
> alloc_size = roundup_pow_of_two(size).  Per include/linux/log2.h the
> result is undefined for size == 0.  The runtime behaviour is
> architecture-dependent:
>
>   * x86, arm64, RISC-V, MIPS, s390x, LoongArch: the ISA shift
>     instruction masks the shift count by (word_bits - 1), so
>     1UL << word_bits evaluates to 1 rather than 0.
>     kmalloc_array(1, sizeof(u16)) returns a valid 2-byte slab
>     allocation with seq_list->mask == 0.  ERTM retransmission
>     silently collapses every reqseq onto slot 0 (correctness bug,
>     no memory corruption).
>
>   * ARMv7 (AArch32), PowerPC 32-bit (slw), PowerPC 64-bit (sld):
>     the shift instruction returns 0 for shift counts >= word
>     width.  1UL << word_bits therefore evaluates to 0,
>     kmalloc_array(0, sizeof(u16)) returns ZERO_SIZE_PTR, and
>     seq_list->mask becomes ULONG_MAX.  Any subsequent access to
>     seq_list->list dereferences ZERO_SIZE_PTR (0x10), which is
>     always an unmapped low-memory address, and the kernel Oopses.
>     This is a remote kernel panic driven by a single peer-sent
>     CONFIG_REQ; it is not a demonstrated code-execution primitive.
>
> Verified on qemu-system-arm -M virt -cpu cortex-a15 (ARMv7-A, same
> LSL register-shift semantics as the Cortex-A9 class still shipping
> in automotive infotainment on NXP i.MX6 with long-term availability
> through 2028).  A KASAN-inline kernel built from mainline panics in
> l2cap_seq_list_init on the first peer CONFIG_REQ carrying
> mode = L2CAP_MODE_ERTM and txwin_size = 0:
>
>   Unable to handle kernel paging request at virtual address
>   9f000002 when read
>   Internal error: Oops: 5 [#1] SMP ARM
>   PC is at l2cap_seq_list_init+0x140/0x28c
>   r2 : 00000010   r1 : ffffffff
>   Register r2 information: zero-size pointer
>   Mode SVC_32  ISA ARM
>   Call trace:
>    l2cap_seq_list_init from l2cap_ertm_init+0x588/0x758
>    l2cap_ertm_init    from l2cap_config_rsp+0xeac/0x1158
>    l2cap_config_rsp   from l2cap_recv_frame+0x1260/0x8000
>    l2cap_recv_frame   from l2cap_recv_acldata+0xb78/0xdb0
>    l2cap_recv_acldata from hci_rx_work
>
> r2 = 0x10 is ZERO_SIZE_PTR (the kernel's own decoder annotates it
> as such).  r1 = 0xFFFFFFFF is seq_list->mask after the 0 - 1
> underflow.  Faulting address 0x9f000002 is the KASAN shadow for
> pointer 0x10 (shadow_offset 0x9f000000 + (0x10 >> 3)).
>
> Trigger is one peer-supplied CONFIG_REQ on an L2CAP ERTM channel;
> no local privileges required, no pairing required, no local
> interaction beyond being within BR/EDR radio range of an affected
> host.
>
> Fix in two places:
>
>   * l2cap_parse_conf_req(): when the peer sends txwin_size = 0 in
>     the RFC option, clamp it up to L2CAP_DEFAULT_TX_WINDOW before
>     the chan->remote_tx_win assignment.  This matches the existing
>     clamp on the CONF_EWS_RECV branch in the same function and
>     mirrors the shape of commit 25f420a0d4cf ("Bluetooth: L2CAP:
>     Fix ERTM re-init and zero pdu_len infinite loop") for the
>     sibling RFC max_pdu_size field.  This is the primary fix: it
>     prevents zero from ever reaching l2cap_seq_list_init() on the
>     normal config path.
>
>   * l2cap_seq_list_init(): return -EINVAL on size == 0 as a
>     defence-in-depth check for any current or future caller that
>     might pass an unclamped value.  The existing error-propagation
>     in l2cap_ertm_init() and its callers already tears the channel
>     down on error, so the peer simply loses the ERTM channel
>     rather than silently corrupting an unmapped ZERO_SIZE_PTR
>     allocation.
>
> Related but distinct from commit 25f420a0d4cf ("Bluetooth: L2CAP:
> Fix ERTM re-init and zero pdu_len infinite loop") which addressed
> the sibling zero-value issue on the RFC max_pdu_size field.
>
> Fixes: 3c588192b5e5 ("Bluetooth: Add the l2cap_seq_list structure for tracking frames")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-7
> ---
>  net/bluetooth/l2cap_core.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 95c65fece39b..b2fe094263ca 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -323,6 +323,17 @@ static int l2cap_seq_list_init(struct l2cap_seq_list *seq_list, u16 size)
>  {
>         size_t alloc_size, i;
>
> +       /*
> +        * A peer may send an ERTM RFC option with txwin_size = 0, which
> +        * propagates here as size = 0.  roundup_pow_of_two(0) is
> +        * documented UB (see include/linux/log2.h) and produces a
> +        * semantically broken seq_list that silently drops every
> +        * retransmission slot.  Reject size = 0 explicitly so the caller
> +        * (l2cap_ertm_init) tears the channel down cleanly instead.
> +        */
> +       if (!size)
> +               return -EINVAL;
> +
>         /* Allocated size is a power of 2 to map sequence numbers
>          * (which may be up to 14 bits) in to a smaller array that is
>          * sized for the negotiated ERTM transmit windows.
> @@ -3593,6 +3604,17 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
>                         break;
>
>                 case L2CAP_MODE_ERTM:
> +                       /*
> +                        * Peer-supplied RFC txwin_size = 0 is out of spec
> +                        * (Core Spec v5.3 Vol 3 Part A 5.4: ERTM tx window
> +                        * range is 1..63, or 1..0x3fff with EWS).  Clamp up
> +                        * to the default window so the subsequent
> +                        * l2cap_seq_list_init(remote_tx_win) does not
> +                        * receive a zero size.
> +                        */
> +                       if (!rfc.txwin_size)
> +                               rfc.txwin_size = L2CAP_DEFAULT_TX_WINDOW;
> +
>                         if (!test_bit(CONF_EWS_RECV, &chan->conf_state))
>                                 chan->remote_tx_win = rfc.txwin_size;
>                         else
> --
> 2.53.0

https://sashiko.dev/#/patchset/20260417221628.1674866-1-michael.bommarito%40gmail.com

We should probably fix the double free if it can occur; perhaps the
others should be addressed in different patches.


-- 
Luiz Augusto von Dentz