* Re: [PATCH net v2 01/10] rxrpc: input: reject ACKALL outside transmit phase
2026-06-18 13:47 ` [PATCH net v2 01/10] rxrpc: input: reject ACKALL outside transmit phase David Howells
@ 2026-06-19 21:32 ` Jeffrey E Altman
0 siblings, 0 replies; 2+ messages in thread
From: Jeffrey E Altman @ 2026-06-19 21:32 UTC (permalink / raw)
To: David Howells, netdev
Cc: Marc Dionne, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, linux-afs, linux-kernel, Wyatt Feng,
stable, Yuan Tan, Yifan Wu, Juefei Pu, Zhengchuan Liang, Xin Liu,
Ren Wei
On 6/18/2026 9:47 AM, David Howells wrote:
> From: Wyatt Feng <bronzed_45_vested@icloud.com>
>
> 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 therefore reach a new service call in
> RXRPC_CALL_SERVER_RECV_REQUEST before any reply packets have been
> queued.
>
> In that state call->tx_top is zero and call->tx_queue is NULL, so
> rxrpc_rotate_tx_window() dereferences a NULL txqueue and triggers a
> null-pointer dereference.
>
> Fix rxrpc_input_ackall() to mirror the transmit-state gating already
> used for normal ACK processing, and ignore ACKALL when there is no
> outstanding transmit window to rotate.
>
> Fixes: b341a0263b1b ("rxrpc: Implement progressive transmission queue struct")
> Cc: stable@vger.kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Assisted-by: Codex:GPT-5.4
> Signed-off-by: Wyatt Feng <bronzed_45_vested@icloud.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Marc Dionne <marc.dionne@auristor.com>
> cc: linux-afs@lists.infradead.org
> ---
> net/rxrpc/input.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
> index ce761466b02d..37881dffa898 100644
> --- a/net/rxrpc/input.c
> +++ b/net/rxrpc/input.c
> @@ -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))
> rxrpc_end_tx_phase(call, false, rxrpc_eproto_unexpected_ackall);
> }
>
Wyatt,
Thank you for identifying the NULL pointer dereference but I do not
believe the patch is correct from an RxRPC protocol perspective.
The rxrpc protocol is not formally standardized. Linux rxrpc is a clean
room implementation of Transarc/IBM RxRPC protocol used by AFS 3.0.
I've been spelunking through old source code trees dating back to
mid-1988. The original usage of the ACKALL packet was a form of delayed
acknowledgement only to be sent after all of the DATA packets inclusive
of the LAST_PACKET had been received. Your expectation of how the
packet type is intended to be used is consistent with that behavior.
However, in Nov 1988 the DATA acknowledgement logic was altered in a
backward incompatible manner. Instead of immediately sending ACK
packets in response to every DATA packet except when the final DATA
packet inclusive of LAST_PACKET was received, the ACK packet usage was
extended to permit delayed transmissions. From Nov 1988 onward ACK
packets were scheduled to be sent with a 200ms delay unless the received
DATA packet was a duplicate, out-of-sequence, out-of-window, etc OR
unless the received DATA packet had the RX_REQUEST_ACK flag set. The
delayed ACKs replaced the ACKALL usage in the general case.
But it appears there was a bug introduced which resulted in the sending
of arbitrary ACKALL packets at any point in the call lifetime. This
bug was not identified until Nov 2001 [OpenAFS
db2ddfaf1b322710e1bd4edce6d7519157c3c9eb] at which point the sending of
ACKALL packets was further restricted. One of the reasons why the
sending of ACKALL packets at arbitrary times was not identified as a
problem for more than a decade is that ACKALL packets received when
there were no transmitted packets waiting for acknowledgement had no
impact on the call state. If there were transmitted packets waiting
for acknowledgement and they were successfully delivered, then the call
continued successfully.
OpenAFS 1.6 pre-releases attempted to resume use of ACKALL packets as a
performance enhancement only to revert the change because of
compatibility problems.
I think the best change at this point would to accept the ACKALL packets
without generating an error regardless of the call state. If there are
transmitted DATA packets waiting for acknowledgement, acknowledge them.
If there are DATA packets which have yet to be sent, leave them alone.
Only complete the call in response to an ACKALL if the ACKALL is
received by the acceptor (incoming call) and all DATA packets inclusive
of LAST_PACKET have been transmitted at least once.
Sincerely,
Jeffrey Altman
^ permalink raw reply [flat|nested] 2+ messages in thread