All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Bommarito <michael.bommarito@gmail.com>
To: David Howells <dhowells@redhat.com>,
	Marc Dionne <marc.dionne@auristor.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>,
	linux-afs@lists.infradead.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: [PATCH] rxrpc: Fix read+write past skb_headlen in soft-ACK parser
Date: Wed, 13 May 2026 14:09:07 -0400	[thread overview]
Message-ID: <20260513180907.2061972-1-michael.bommarito@gmail.com> (raw)

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


             reply	other threads:[~2026-05-13 18:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 18:09 Michael Bommarito [this message]
2026-05-15 13:49 ` [PATCH] rxrpc: Fix read+write past skb_headlen in soft-ACK parser Jeffrey E Altman
2026-05-15 23:17   ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260513180907.2061972-1-michael.bommarito@gmail.com \
    --to=michael.bommarito@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.