From: Jeffrey E Altman <jaltman@auristor.com>
To: David Howells <dhowells@redhat.com>, netdev@vger.kernel.org
Cc: Marc Dionne <marc.dionne@auristor.com>,
Jakub Kicinski <kuba@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
linux-afs@lists.infradead.org, linux-kernel@vger.kernel.org,
Wyatt Feng <bronzed_45_vested@icloud.com>,
stable@vger.kernel.org, Yuan Tan <yuantan098@gmail.com>,
Yifan Wu <yifanwucs@gmail.com>,
Juefei Pu <tomapufckgml@gmail.com>,
Zhengchuan Liang <zcliangcn@gmail.com>, Xin Liu <bird@lzu.edu.cn>,
Ren Wei <n05ec@lzu.edu.cn>
Subject: Re: [PATCH net v2 01/10] rxrpc: input: reject ACKALL outside transmit phase
Date: Fri, 19 Jun 2026 17:32:38 -0400 [thread overview]
Message-ID: <c0fd4fec-1576-4070-b31e-a37d5506f5ed@auristor.com> (raw)
In-Reply-To: <20260618134802.2477777-2-dhowells@redhat.com>
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
next prev parent reply other threads:[~2026-06-19 21:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-18 13:47 [PATCH net v2 00/10] rxrpc: Miscellaneous fixes David Howells
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 [this message]
2026-06-18 13:47 ` [PATCH net v2 02/10] rxrpc: Fix leak of connection from OOB challenge David Howells
2026-06-18 13:47 ` [PATCH net v2 03/10] rxrpc: Fix double unlock in rxrpc_recvmsg() David Howells
2026-06-18 13:47 ` [PATCH net v2 04/10] afs: Fix further netns teardown to cancel the preallocation charger David Howells
2026-06-18 13:47 ` [PATCH net v2 05/10] afs: Fix uncancelled rxrpc OOB message handler David Howells
2026-06-20 9:13 ` Simon Horman
2026-06-18 13:47 ` [PATCH net v2 06/10] rxrpc: Fix the reception of a reply packet before data transmission David Howells
2026-06-20 9:17 ` Simon Horman
2026-06-18 13:47 ` [PATCH net v2 07/10] rxrpc: Fix oob challenge leak in cleanup after notification failure David Howells
2026-06-20 9:17 ` Simon Horman
2026-06-18 13:47 ` [PATCH net v2 08/10] rxrpc: Fix potential infinite loop in rxrpc_recvmsg() David Howells
2026-06-18 13:48 ` [PATCH net v2 09/10] rxrpc: Fix socket notification race David Howells
2026-06-18 13:48 ` [PATCH net v2 10/10] rxrpc: Fix leak of released call in recvmsg(MSG_PEEK) 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=c0fd4fec-1576-4070-b31e-a37d5506f5ed@auristor.com \
--to=jaltman@auristor.com \
--cc=bird@lzu.edu.cn \
--cc=bronzed_45_vested@icloud.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=n05ec@lzu.edu.cn \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=stable@vger.kernel.org \
--cc=tomapufckgml@gmail.com \
--cc=yifanwucs@gmail.com \
--cc=yuantan098@gmail.com \
--cc=zcliangcn@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox