Re: [PATCH net] net/rds: tcp: bound the message length announced by the peer

[Allison Henderson <achender@kernel.org>]

On Wed, 2026-11-11 at 14:27 -0600, Weidong Wang via B4 Relay wrote:
> From: Weidong Wang <weidongw@illinois.edu>
> 
> rds_tcp_data_recv() takes the payload length of an incoming message from
> the peer's header and queues pskb_extract() clones of the received data
> on the incoming message until that many bytes have arrived. Nothing
> checks the length, so a peer that announces h_len = 0xffffffff and keeps
> sending makes the receiver hold up to 4 GiB per connection. The clones
> are no longer on the socket's receive queue, so they are not charged to
> the socket and TCP keeps the receive window open.
> 
> Senders are limited to RDS_MAX_MSG_SIZE in rds_sendmsg(). Apply the same
> limit on receive and drop the connection when a header exceeds it. That
> limit has been enforced on send since v4.11; a peer running an older
> kernel could still send larger messages over TCP, and such connections
> are now dropped.
> 
> The issue was discovered by an AI code review agent (an LLM-based
> bug-finding tool developed by Zijie Zhao and the submitter) and
> confirmed by triggering it on an unmodified kernel, without error
> injection, on x86_64 under QEMU: a plain TCP client in a second network
> namespace connects to the rds_tcp listener (port 16385), sends one RDS
> header with h_len = 0xffffffff and then 1 GiB of payload. The receiver
> held about 1.1 GiB until the connection was closed; the same 1 GiB sent
> as 1 MiB messages held about 60 MiB.
> 
> With this patch the connection is dropped right after the header and
> nothing is held, while the 1 MiB messages are processed as before. The
> RDS selftest (tools/testing/selftests/net/rds, rds_run.sh -T tcp, also
> with -l 10 -c 10 -u 10) passes with the patch applied.
> 
> The reproducer, fix and changelog were drafted with AI assistance
> (Claude), then reviewed and tested by the submitter.
> 
> Fixes: 70041088e3b9 ("RDS: Add TCP transport to RDS")
> Cc: stable@vger.kernel.org
> Cc: Zijie Zhao <zijie4@illinois.edu>
> Signed-off-by: Weidong Wang <weidongw@illinois.edu>

Hi Weidong,

Thanks for the catch.  This patch looks fine to me.
Reviewed-by: Allison Henderson <achender@kernel.org>

Thanks!
Allison

> ---
> Tested on v7.4-rc1 (05441d697f7f) and on net (ae89f2724e), x86_64,
> KASAN, QEMU/KVM, 3 GiB guest. Setup: rds_tcp listener in the initial
> netns (10.0.0.2); a plain TCP client (no AF_RDS) in a second netns
> (10.0.0.1) over a veth pair.
> 
>                    1 GiB after one header     1 GiB as 1 MiB
>                    with h_len = 0xffffffff    messages (control)
>   unpatched        ~1.1 GiB held until close  ~60 MiB
>   patched          connection reset after     ~60 MiB, processed
>                    the header, nothing held   as before
> 
> selftests/net/rds: rds_run.sh -T tcp and rds_run.sh -T tcp -l 10 -c 10
> -u 10 pass on both trees with the patch. No KASAN/UBSAN reports;
> checkpatch clean.
> 
> The limit is per connection; a peer with several source addresses gets
> one connection per address. The reproducer is not attached (AI-found
> issue, see security-bugs.rst) and can be shared on request.
> ---
>  net/rds/tcp_recv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/rds/tcp_recv.c b/net/rds/tcp_recv.c
> index ffe843ca2..29db73df8 100644
> --- a/net/rds/tcp_recv.c
> +++ b/net/rds/tcp_recv.c
> @@ -213,6 +213,20 @@ static int rds_tcp_data_recv(read_descriptor_t *desc, struct sk_buff *skb,
>  			}
>  		}
>  
> +		/*
> +		 * The payload is queued on tinc until the whole message has
> +		 * arrived, so a peer must not announce more than a sender is
> +		 * allowed to send. Check on every pass, not only when the
> +		 * header completes: the drop is asynchronous and more data
> +		 * can arrive before the connection is shut down.
> +		 */
> +		if (!tc->t_tinc_hdr_rem &&
> +		    tc->t_tinc_data_rem > RDS_MAX_MSG_SIZE) {
> +			rds_conn_path_drop(cp, false);
> +			desc->error = -EPROTO;
> +			goto out;
> +		}
> +
>  		if (left && tc->t_tinc_data_rem) {
>  			to_copy = min(tc->t_tinc_data_rem, left);
>  
> 
> ---
> base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
> change-id: 20261110-h047-send-52025d442823
> 
> Best regards,
> --  
> Weidong Wang <weidongw@illinois.edu>
> 
>