Re: [PATCH net v2 06/10] rxrpc: Fix the reception of a reply packet before data transmission

[Simon Horman <horms@kernel.org>]

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: Fix the reception of a reply packet before data transmission

This commit fixes rxrpc_receiving_reply() to handle the reception of an
apparent reply DATA packet before rxrpc has had a chance to send any request
DATA packets. It also adjusts rxrpc_rotate_tx_window() to handle an empty
Tx queue by changing the do...while loop into a while loop.

Link: https://sashiko.dev/#/patchset/20260616155749.2125907-1-dhowells%40redhat.com

> diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
> index 37881dffa898b..01ccd2d2fe92c 100644
> --- a/net/rxrpc/input.c
> +++ b/net/rxrpc/input.c
> @@ -247,7 +247,7 @@ static bool rxrpc_rotate_tx_window(struct rxrpc_call *call, rxrpc_seq_t to,
>  		tq = call->tx_queue;
>  	}
>  
> -	do {
> +	while (before_eq(seq, to)) {

[Severity: Critical]
This is a pre-existing issue, but can this trigger a use-after-free write
during a concurrent sendmsg() if the remote peer sends an early reply?

Right before this loop in rxrpc_rotate_tx_window(), the code unconditionally
frees the active transmission queue buffer:

	if (seq == call->tx_qbase + RXRPC_NR_TXQUEUE) {
		call->tx_qbase += RXRPC_NR_TXQUEUE;
		call->tx_queue = tq->next;
		...
		kfree(tq);
		tq = call->tx_queue;
	}

If a full block of 63 packets is transmitted and ACKed, call->tx_bottom
reaches 63. If the remote peer then sends an early reply,
rxrpc_receiving_reply() calls rxrpc_rotate_tx_window() with to = 63 and seq
starting at 64.

This triggers the condition above and frees the active block (tq) without
verifying if it is still being written to. If it is still the tail
(tq == call->tx_qtail), call->send_queue and call->tx_qtail are left as
dangling pointers.

Concurrently, if the client is still sending data via sendmsg(), it might call
rxrpc_alloc_txqueue() for the 64th packet:

net/rxrpc/sendmsg.c:rxrpc_alloc_txqueue() {
	...
	if (call->send_queue) {
		tq->qbase = call->send_top + 1;
		call->send_queue->next = tq;
		call->send_queue = tq;
	}
	...
}

Will call->send_queue->next = tq write a newly allocated pointer into the
recently freed memory block?

[ ... ]