public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: L2CAP: handle zero txwin_size in ERTM RFC option
@ 2026-04-17 22:16 Michael Bommarito
  2026-04-17 22:55 ` bluez.test.bot
  2026-04-20 19:06 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 7+ messages in thread
From: Michael Bommarito @ 2026-04-17 22:16 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Mat Martineau, Hyunwoo Kim, linux-bluetooth, linux-kernel, stable

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@vger.kernel.org
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
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


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

end of thread, other threads:[~2026-04-21 14:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 22:16 [PATCH] Bluetooth: L2CAP: handle zero txwin_size in ERTM RFC option Michael Bommarito
2026-04-17 22:55 ` bluez.test.bot
2026-04-20 19:06 ` [PATCH] " Luiz Augusto von Dentz
2026-04-21 13:56   ` [PATCH v2 0/2] Bluetooth: L2CAP: fix zero txwin_size handling and repeated CONFIG_RSP re-init Michael Bommarito
2026-04-21 13:56   ` [PATCH v2 1/2] Bluetooth: L2CAP: handle zero txwin_size in ERTM RFC option Michael Bommarito
2026-04-21 14:51     ` [v2,1/2] " bluez.test.bot
2026-04-21 13:56   ` [PATCH v2 2/2] Bluetooth: L2CAP: skip ERTM re-init on repeated CONFIG_RSP Michael Bommarito

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