On 5/13/2026 2:09 PM, Michael Bommarito wrote: > 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 > --- > 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); > } > Aborting the call because skb_condense() was unable to consolidate the rx ack packet data is an unfriendly thing to do. As suggested by the commit message, copying the data before processing would be a friendlier solution to the identified problem. Jeffrey Altman