Netdev List
 help / color / mirror / Atom feed
* [PATCH] rxrpc: Fix read+write past skb_headlen in soft-ACK parser
@ 2026-05-13 18:09 Michael Bommarito
  0 siblings, 0 replies; only message in thread
From: Michael Bommarito @ 2026-05-13 18:09 UTC (permalink / raw)
  To: David Howells, Marc Dionne, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, linux-afs, netdev, linux-kernel, stable

rxrpc_input_soft_acks() builds a raw `u8 *acks = skb->data + ...`
pointer and walks it for `sp->ack.nr_acks` iterations, performing a
read-modify-write (shiftr_adv_rotr) on each byte.

The caller rxrpc_input_ack() only validates that the bytes exist
somewhere in the skb (`offset > skb->len - nr_acks`) and best-effort
linearises the head with skb_condense().  skb_condense() returns
without pulling when the skb is cloned, when paged data exceeds the
linear-head tailroom, or when frags are unreadable.  On a nonlinear
skb that survives the condense step (cloned by AF_PACKET capture,
frag_list-style after IP-fragment reassembly, or paged-frag receive
on real NICs), skb->data covers only the linear head.  The parser
then walks past skb_headlen(skb) into skb tailroom, skb_shared_info,
or the next slab object, doing in-place 1-byte shifts on up to 255
attacker-controlled offsets per ACK packet.

Sibling parsers in the same file already use the safe pattern:
rxrpc_extract_header(), rxrpc_extract_abort(), rxrpc_input_split_jumbo(),
and the rxrpc_input_ack_trailer() call site all use skb_copy_bits()
with explicit length checks.  The soft-ACK call path is the lone
direct-deref site.

Add an explicit pskb_may_pull() check before invoking the parser so
that the linear head is guaranteed to cover the SACK bitmap.  On
allocation failure return rxrpc_proto_abort() with the same
eproto_ackr_short_sack disposition the existing length check uses.
skb_condense() is retained on the path; its truesize-accounting side
effect is independent of the linearisation guarantee that
pskb_may_pull() now provides.

The bug shape was reproduced under UML+KASAN in two complementary
harnesses:

(1) A kmod that lifts the parser's inner shift loop verbatim and
    exercises it against a kmalloc(47) buffer.  KASAN reports a
    slab-out-of-bounds read on the first byte past the allocation:

      BUG: KASAN: slab-out-of-bounds in run_rxrpc_soft_acks_loop+0x52/0x74
      Read of size 1 at addr 63a7032f by task insmod/37
       which belongs to the cache kmalloc-64 of size 64
       allocated 47-byte region [63a70300, 63a7032f)

(2) A second kmod uses the in-kernel rxrpc API to allocate a real
    rxrpc_call, builds a nonlinear hostile ACK skb (linear head=46,
    paged frag=79, skb->cloned=1, nr_acks=60) and drives the
    upstream rxrpc_input_call_packet() -> rxrpc_input_ack() ->
    rxrpc_input_soft_acks() chain directly.  Sixty 0xAA sentinel
    bytes placed in the linear-head tailroom are all right-shifted
    to 0x55 by the unmodified upstream rxrpc_input_soft_acks() on
    a stock kernel.  On the patched kernel, zero of sixty shift --
    pskb_may_pull aborts the call before the parser runs.

Note: the real-path demonstration does NOT produce a literal
KASAN slab-out-of-bounds splat, because the on-wire nAcks field
is a u8 (max 255) and the OOB shift stays within the same kmalloc
slab object that holds skb_shared_info.  Per-byte corruption of
skb_shared_info and the linear-head tailroom is the actual
production effect.

A regression check on a fully-linear ACK skb confirms pskb_may_pull
is a no-op on that path; the parser continues to read in-bounds.

Fixes: d57a3a151660 ("rxrpc: Save last ACK's SACK table rather than marking txbufs")
Cc: stable@vger.kernel.org
Reported via internal source-audit pipeline on 2026-04-21.
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 net/rxrpc/input.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 24aceb183c2c..52ace0f98d06 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -1173,6 +1173,8 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
 	if (nr_acks > 0) {
 		if (offset > (int)skb->len - nr_acks)
 			return rxrpc_proto_abort(call, 0, rxrpc_eproto_ackr_short_sack);
+		if (!pskb_may_pull(skb, offset + nr_acks))
+			return rxrpc_proto_abort(call, 0, rxrpc_eproto_ackr_short_sack);
 		rxrpc_input_soft_acks(call, &summary, skb);
 	}
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-05-13 18:09 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 18:09 [PATCH] rxrpc: Fix read+write past skb_headlen in soft-ACK parser Michael Bommarito

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