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

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

[Weidong Wang via B4 Relay]

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>
---
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>

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

[Allison Henderson]

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>
> 
> 

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] After the new check in rds_tcp_data_recv() fires, the receive
  path stops consuming data altogether, and this breaks the drain loop…

Pre-existing issues:
- [High] The patch does not cover the RDS IB/RDMA receive path, which has
  the same flaw it fixes for TCP: the peer-announced h_len is never…

--

net/rds: tcp: bound the message length announced by the peer

This patch adds a check to rds_tcp_data_recv(). Once the header is
complete, the h_len announced by the peer is compared against
RDS_MAX_MSG_SIZE. If it is larger, the connection path is dropped and the
receive actor returns with -EPROTO.

> diff --git a/net/rds/tcp_recv.c b/net/rds/tcp_recv.c
> index ffe843ca219c7..29db73df84fa3 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;
> +		}

[Severity: Medium]
After this check fires, tc->t_tinc is still set, with t_tinc_hdr_rem == 0
and t_tinc_data_rem > RDS_MAX_MSG_SIZE. Won't every later call into
rds_tcp_data_recv() hit the same check straight away and return 0 without
consuming anything?

__tcp_read_sock() stops on a 0 return:

    used = recv_actor(desc, skb, offset, len);
    if (used <= 0) {
        if (!copied)
            copied = used;
        break;
    }

So any payload after the oversized header stays on sk_receive_queue.

The drain loop in rds_tcp_conn_path_shutdown() only exits early when that
queue is empty:

    rds_tcp_recv_path(cp);
    } while (!wait_event_timeout(tc->t_recv_done_waitq,
            (sk->sk_state == TCP_CLOSING ||
             ...
             sk->sk_state == TCP_CLOSE) &&
            skb_queue_empty_lockless(&sk->sk_receive_queue),
            msecs_to_jiffies(100)) &&
         ++rounds < 50);

tc->t_tinc is only reset after this loop. Does that mean the loop now
always runs the full 50 x 100ms (about 5 seconds), even when the peer
answers our FIN promptly?

Each round goes through this chain and calls rds_conn_path_drop() again:

  rds_tcp_recv_path()->rds_tcp_read_sock()->tcp_read_sock()->rds_tcp_data_recv()

That call does atomic_set(&cp->cp_state, RDS_CONN_ERROR) while the path is
in RDS_CONN_DISCONNECTING, and it re-queues cp_down_w. The
DISCONNECTING->DOWN transition in rds_conn_shutdown() then fails and
returns early. Is a second full shutdown pass forced before the reconnect?

This holds up the path's ordered workqueue, or the shared rds_wq if
alloc_ordered_workqueue() failed, and it delays reconnect. It would also
apply to every reconnect cycle of the old-kernel peers that the changelog
says will now be dropped.

Would it work better to consume and discard the rest of the stream once
the protocol error is detected, by returning the full len without cloning?
The drain loop could then finish, and the drop would not be repeated.

[Severity: High]
This isn't a bug introduced by this patch, but the IB transport appears to
have the same unbounded h_len problem. In net/rds/ib_recv.c,
rds_ib_process_recv() does:

    memcpy(hdr, ihdr, sizeof(*hdr));
    ic->i_recv_data_rem = be32_to_cpu(hdr->h_len);

There is no RDS_MAX_MSG_SIZE check. Each fragment is then kept until
i_recv_data_rem reaches 0:

    list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);

Continuation fragments are only checked for a matching header and a
minimum length. rds_ib_refill_one_frag() allocates a replacement frag for
each one consumed, with no limit.

The rds_ib_sysctl_max_recv_allocation cap in rds_ib_refill_one_inc()
counts rds_ib_incoming structures. A single huge message uses only one of
those.

Can an IB/RoCE peer that sends h_len = 0xffffffff, followed by full-sized
fragments, make the receiver hold up to about 4 GiB per connection?

rds.h describes RDS_MAX_MSG_SIZE as the limit for "both RDMA and non-RDMA
RDS message". Should rds_ib_process_recv() get an equivalent check,
followed by rds_ib_conn_error()?

Since this is tagged for stable, would it also help for the changelog to
say that only the TCP receive path is bounded?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111-h047-send-v1-1-2727a38dbfcd%40illinois.edu