Re: [PATCH net 1/5] rxrpc: input: reject ACKALL outside transmit phase
From: Simon Horman <horms@kernel.org>
Date: 2026-06-18 09:28:59
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- rxrpc: input: reject ACKALL outside transmit phase rxrpc_input_ackall() accepts ACKALL packets without checking whether the call is in a state that can legitimately have outstanding transmit buffers. A forged ACKALL can reach a new service call and trigger a null pointer dereference.
quoted hunk ↗ jump to hunk
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c index ce761466b02d6..37881dffa898b 100644 --- a/net/rxrpc/input.c +++ b/net/rxrpc/input.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1214,8 +1214,22 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb) static void rxrpc_input_ackall(struct rxrpc_call *call, struct sk_buff *skb) { struct rxrpc_ack_summary summary = { 0 }; + rxrpc_seq_t top = READ_ONCE(call->tx_top); + + switch (__rxrpc_call_state(call)) { + case RXRPC_CALL_CLIENT_SEND_REQUEST: + case RXRPC_CALL_CLIENT_AWAIT_REPLY: + case RXRPC_CALL_SERVER_SEND_REPLY: + case RXRPC_CALL_SERVER_AWAIT_ACK: + break; + default: + return; + } + + if (call->tx_bottom == top) + return; - if (rxrpc_rotate_tx_window(call, call->tx_top, &summary)) + if (rxrpc_rotate_tx_window(call, top, &summary))
[Severity: High]
This isn't a bug introduced by this patch, but it appears the fix might be
incomplete. The patch prevents a NULL pointer dereference in
rxrpc_input_ackall() by checking if call->tx_bottom == top before calling
rxrpc_rotate_tx_window().
However, does the same vulnerability remain in rxrpc_receiving_reply()?
If a client in the RXRPC_CALL_CLIENT_SEND_REQUEST state receives a forged or
early DATA packet (acting as a reply) before it has queued any transmit data,
call->tx_top and call->tx_bottom will both be 0, and call->tx_queue will be
NULL.
rxrpc_receiving_reply() will then unconditionally call:
if (!rxrpc_rotate_tx_window(call, top, &summary)) {
Because rxrpc_rotate_tx_window() uses a do { ... } while (before_eq(seq, to))
loop, it executes exactly once for seq = 1 and to = 0, which dereferences
the NULL tq:
if (tq->bufs[ix]->flags & RXRPC_LAST_PACKET) {
Should the empty window check be placed at the top of rxrpc_rotate_tx_window()
instead, or mirrored in rxrpc_receiving_reply() to protect against early
replies?
rxrpc_end_tx_phase(call, false, rxrpc_eproto_unexpected_ackall); }