[PATCH net v3 01/11] rxrpc: Fix ACKALL packet handling

[David Howells <dhowells@redhat.com>]

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 the handling of ACKALL packets by the following means:

 (1) Add two new call states: RXRPC_CALL_CLIENT_PRE_SEND which indicates
     that the client call is connected, but nothing has been transmitted as
     yet; and RXRPC_CALL_CLIENT_AWAIT_ACK, which indicates that everything
     has been transmitted at least once, but we're now waiting for the
     stuff remaining in the Tx buffer to be ACK'd (retransmissions may
     still happen).

     The RXRPC_CALL_CLIENT_PRE_SEND state is set when the call is assigned
     a channel and transitions to RXRPC_CALL_CLIENT_SEND_REQUEST when the
     first packet is transmitted.

     RXRPC_CALL_CLIENT_AWAIT_REPLY is then narrowed in scope to indicate
     that all Tx packets have been ACK'd and we're now waiting for the
     reply to be received.

 (2) As per Wyatt Feng's original patch[1], the ACKALL handler then checks
     that the call state is one in which there might be stuff in the Tx
     buffer to ACK, but now this includes AWAIT_ACK rather than
     AWAIT_REPLY.  ACKALL packets are ignored if received in the wrong
     state.

     Note that unlike Wyatt Feng's patch, it's no longer necessary to check
     to see if the Tx buffer exists as this the state set now covers this.

 (3) Make the ACKALL handler use call->tx_transmitted rather than
     call->tx_top as the former is explicitly the highest packet seq number
     transmitted, whereas the latter has a looser definition.

Thanks to Jeffrey Altman for a description of the history of the ACKALL
packet[1].

Fixes: b341a0263b1b ("rxrpc: Implement progressive transmission queue struct")
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>
Signed-off-by: Wyatt Feng <bronzed_45_vested@icloud.com>
Co-developed-by: David Howells <dhowells@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Ren Wei <n05ec@lzu.edu.cn>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20260616155749.2125907-2-dhowells@redhat.com/ [1]
Link: https://lore.kernel.org/r/c0fd4fec-1576-4070-b31e-a37d5506f5ed@auristor.com/ [2]
---
 net/rxrpc/ar-internal.h |  2 ++
 net/rxrpc/call_event.c  |  5 ++++-
 net/rxrpc/call_object.c |  2 ++
 net/rxrpc/conn_client.c |  2 +-
 net/rxrpc/input.c       | 23 +++++++++++++++++++----
 net/rxrpc/sendmsg.c     |  3 ++-
 6 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 98f2165159d7..b6ccd8a8199b 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -650,7 +650,9 @@ enum rxrpc_call_event {
 enum rxrpc_call_state {
 	RXRPC_CALL_UNINITIALISED,
 	RXRPC_CALL_CLIENT_AWAIT_CONN,	/* - client waiting for connection to become available */
+	RXRPC_CALL_CLIENT_PRE_SEND,	/* - client is connected, but hasn't sent anything yet */
 	RXRPC_CALL_CLIENT_SEND_REQUEST,	/* - client sending request phase */
+	RXRPC_CALL_CLIENT_AWAIT_ACK,	/* - client awaiting ACKs of request */
 	RXRPC_CALL_CLIENT_AWAIT_REPLY,	/* - client awaiting reply */
 	RXRPC_CALL_CLIENT_RECV_REPLY,	/* - client receiving reply phase */
 	RXRPC_CALL_SERVER_PREALLOC,	/* - service preallocation */
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index fec59d9338b9..21be9c86d7a7 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -178,7 +178,7 @@ static void rxrpc_close_tx_phase(struct rxrpc_call *call)
 
 	switch (__rxrpc_call_state(call)) {
 	case RXRPC_CALL_CLIENT_SEND_REQUEST:
-		rxrpc_set_call_state(call, RXRPC_CALL_CLIENT_AWAIT_REPLY);
+		rxrpc_set_call_state(call, RXRPC_CALL_CLIENT_AWAIT_ACK);
 		break;
 	case RXRPC_CALL_SERVER_SEND_REPLY:
 		rxrpc_set_call_state(call, RXRPC_CALL_SERVER_AWAIT_ACK);
@@ -244,6 +244,8 @@ static void rxrpc_transmit_fresh_data(struct rxrpc_call *call, unsigned int limi
 				break;
 		} while (req.n < limit && before(seq, send_top));
 
+		if (__rxrpc_call_state(call) == RXRPC_CALL_CLIENT_PRE_SEND)
+			rxrpc_set_call_state(call, RXRPC_CALL_CLIENT_SEND_REQUEST);
 		if (txb->flags & RXRPC_LAST_PACKET) {
 			rxrpc_close_tx_phase(call);
 			tq = NULL;
@@ -267,6 +269,7 @@ void rxrpc_transmit_some_data(struct rxrpc_call *call, unsigned int limit,
 		fallthrough;
 
 	case RXRPC_CALL_SERVER_SEND_REPLY:
+	case RXRPC_CALL_CLIENT_PRE_SEND:
 	case RXRPC_CALL_CLIENT_SEND_REQUEST:
 		if (!rxrpc_tx_window_space(call))
 			return;
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index fcb9d38bb521..817ed9acb91e 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -18,7 +18,9 @@
 const char *const rxrpc_call_states[NR__RXRPC_CALL_STATES] = {
 	[RXRPC_CALL_UNINITIALISED]		= "Uninit  ",
 	[RXRPC_CALL_CLIENT_AWAIT_CONN]		= "ClWtConn",
+	[RXRPC_CALL_CLIENT_PRE_SEND]		= "ClPreSnd",
 	[RXRPC_CALL_CLIENT_SEND_REQUEST]	= "ClSndReq",
+	[RXRPC_CALL_CLIENT_AWAIT_ACK]		= "ClAwtAck",
 	[RXRPC_CALL_CLIENT_AWAIT_REPLY]		= "ClAwtRpl",
 	[RXRPC_CALL_CLIENT_RECV_REPLY]		= "ClRcvRpl",
 	[RXRPC_CALL_SERVER_PREALLOC]		= "SvPrealc",
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 9b757798dedd..48519f0de185 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -449,7 +449,7 @@ static void rxrpc_activate_one_channel(struct rxrpc_connection *conn,
 	trace_rxrpc_connect_call(call);
 	call->tx_last_sent = ktime_get_real();
 	rxrpc_start_call_timer(call);
-	rxrpc_set_call_state(call, RXRPC_CALL_CLIENT_SEND_REQUEST);
+	rxrpc_set_call_state(call, RXRPC_CALL_CLIENT_PRE_SEND);
 	wake_up(&call->waitq);
 }
 
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index ce761466b02d..2eedab1b0919 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -181,7 +181,8 @@ void rxrpc_congestion_degrade(struct rxrpc_call *call)
 	if (call->cong_ca_state != RXRPC_CA_SLOW_START &&
 	    call->cong_ca_state != RXRPC_CA_CONGEST_AVOIDANCE)
 		return;
-	if (__rxrpc_call_state(call) == RXRPC_CALL_CLIENT_AWAIT_REPLY)
+	if (__rxrpc_call_state(call) == RXRPC_CALL_CLIENT_AWAIT_ACK ||
+	    __rxrpc_call_state(call) == RXRPC_CALL_CLIENT_AWAIT_REPLY)
 		return;
 
 	rtt = ns_to_ktime(call->srtt_us * (NSEC_PER_USEC / 8));
@@ -356,6 +357,7 @@ static void rxrpc_end_tx_phase(struct rxrpc_call *call, bool reply_begun,
 
 	switch (__rxrpc_call_state(call)) {
 	case RXRPC_CALL_CLIENT_SEND_REQUEST:
+	case RXRPC_CALL_CLIENT_AWAIT_ACK:
 	case RXRPC_CALL_CLIENT_AWAIT_REPLY:
 		if (reply_begun) {
 			rxrpc_set_call_state(call, RXRPC_CALL_CLIENT_RECV_REPLY);
@@ -694,6 +696,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
 
 	switch (__rxrpc_call_state(call)) {
 	case RXRPC_CALL_CLIENT_SEND_REQUEST:
+	case RXRPC_CALL_CLIENT_AWAIT_ACK:
 	case RXRPC_CALL_CLIENT_AWAIT_REPLY:
 		/* Received data implicitly ACKs all of the request
 		 * packets we sent when we're acting as a client.
@@ -1154,10 +1157,12 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
 	if (hard_ack + 1 == 0)
 		return rxrpc_proto_abort(call, 0, rxrpc_eproto_ackr_zero);
 
-	/* Ignore ACKs unless we are or have just been transmitting. */
+	/* Ignore ACKs unless we are transmitting or are waiting for
+	 * acknowledgement of the packets we've just been transmitting.
+	 */
 	switch (__rxrpc_call_state(call)) {
 	case RXRPC_CALL_CLIENT_SEND_REQUEST:
-	case RXRPC_CALL_CLIENT_AWAIT_REPLY:
+	case RXRPC_CALL_CLIENT_AWAIT_ACK:
 	case RXRPC_CALL_SERVER_SEND_REPLY:
 	case RXRPC_CALL_SERVER_AWAIT_ACK:
 		break;
@@ -1215,7 +1220,17 @@ static void rxrpc_input_ackall(struct rxrpc_call *call, struct sk_buff *skb)
 {
 	struct rxrpc_ack_summary summary = { 0 };
 
-	if (rxrpc_rotate_tx_window(call, call->tx_top, &summary))
+	switch (__rxrpc_call_state(call)) {
+	case RXRPC_CALL_CLIENT_SEND_REQUEST:
+	case RXRPC_CALL_CLIENT_AWAIT_ACK:
+	case RXRPC_CALL_SERVER_SEND_REPLY:
+	case RXRPC_CALL_SERVER_AWAIT_ACK:
+		break;
+	default:
+		return;
+	}
+
+	if (rxrpc_rotate_tx_window(call, call->tx_transmitted, &summary))
 		rxrpc_end_tx_phase(call, false, rxrpc_eproto_unexpected_ackall);
 }
 
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index c35de4fd75e3..ed2c9a51005a 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -366,7 +366,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	if (state >= RXRPC_CALL_COMPLETE)
 		goto maybe_error;
 	ret = -EPROTO;
-	if (state != RXRPC_CALL_CLIENT_SEND_REQUEST &&
+	if (state != RXRPC_CALL_CLIENT_PRE_SEND &&
+	    state != RXRPC_CALL_CLIENT_SEND_REQUEST &&
 	    state != RXRPC_CALL_SERVER_ACK_REQUEST &&
 	    state != RXRPC_CALL_SERVER_SEND_REPLY) {
 		/* Request phase complete for this client call */