[PATCH net v3] net/rds: ib: zero the unwritten tail of a short receive fragment

[PATCH net v3] net/rds: ib: zero the unwritten tail of a short receive fragment

[Shubham Antil]

rds_ib_process_recv() accepts an RDS/IB fragment once the receive
completion reports at least an RDS header, and then trusts the
header-declared message length h_len: rds_ib_inc_copy_to_user() later
copies up to h_len bytes from the fragment pages to userspace on
recvmsg().

When a fragment carries fewer payload bytes than h_len accounts for, the
bytes between the end of the received payload and h_len are read from the
unwritten tail of the fragment page.  That page comes from the per-CPU
receive cache and is not zeroed, so recvmsg() returns stale page contents
to userspace.

Zero the unwritten tail of a short fragment page on receipt, so the copy
can only ever return received payload or zeros.  Fragments that fill the
page are unaffected.

Confirmed under KMSAN: without this change recvmsg() returns stale
frag-page bytes and KMSAN flags an uninitialised copy in
rds_ib_inc_copy_to_user(); with it the same over-read returns zeros and
KMSAN is clean.

Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>
Closes: https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/
Reported-by: Shubham Antil <shubham@octane.security>
Reported-by: Giovanni Vignone <gio@octane.security>
Reported-by: Robert van Eijk <robert@octane.security>
Reported-by: Paolo Gentry <paolo@octane.security>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Shubham Antil <shubham@octane.security>
---
v3:
 - Zero the unwritten fragment tail instead of dropping the connection.
   A zero-copy send with a small iovec segment can legitimately produce a
   non-final fragment shorter than RDS_FRAG_SIZE, so the v2 length check
   could force a reconnect on well-formed traffic; zeroing the tail avoids
   that while still keeping uninitialised bytes out of recvmsg().
   (Raised by the Sashiko review.)
 - Drop Reviewed-by, since the approach changed.
v2:
 - Add Reported-by (sungbyeongchan, first to report, and the Octane
   Security reporters).
 net/rds/ib_recv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index bd6cb3ffa..9695c7f24 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -35,6 +35,7 @@
 #include <linux/slab.h>
 #include <linux/pci.h>
 #include <linux/dma-mapping.h>
+#include <linux/highmem.h>
 #include <rdma/rdma_cm.h>
 
 #include "rds_single_path.h"
@@ -949,6 +950,19 @@ static void rds_ib_process_recv(struct rds_connection *conn,
 		}
 	}
 
+	/* The device wrote only data_len bytes into this recycled fragment
+	 * page.  Zero the unwritten tail so a later rds_ib_inc_copy_to_user(),
+	 * which copies up to the peer-declared h_len, cannot hand stale page
+	 * contents to userspace.
+	 */
+	if (data_len < RDS_FRAG_SIZE) {
+		void *frag = kmap_local_page(sg_page(&recv->r_frag->f_sg));
+
+		memset(frag + recv->r_frag->f_sg.offset + data_len, 0,
+		       RDS_FRAG_SIZE - data_len);
+		kunmap_local(frag);
+	}
+
 	list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
 	recv->r_frag = NULL;
 
-- 
2.43.0

Re: [PATCH net v3] net/rds: ib: zero the unwritten tail of a short receive fragment

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v3] net/rds: ib: zero the unwritten tail of a short receive fragment

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rds: ib: zeroing short non-final fragments corrupts payload and desynchronizes framing
- [High] net/rds: unbounded memory allocation on receive via untrusted h_len
--

commit 155e0b4d269458cc1b33d64be6029fcc1b1efa50
Author: Shubham Antil <shubham@octane.security>
Subject: net/rds: ib: zero the unwritten tail of a short receive fragment

This commit zeroes the unwritten tail of short receive fragments to prevent
stale page contents from being copied to userspace. It attempts to resolve a
KMSAN uninitialized memory warning by ensuring the tail of the fragment is
zeroed before processing.

Closes: https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/

> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> index bd6cb3ffaa571..9695c7f24eaa7 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c

[ ... ]

> @@ -949,6 +950,19 @@ static void rds_ib_process_recv(struct rds_connection *conn,
>  		}
>  	}
>  

[Severity: High]
Does this code allow unbounded memory allocation based on an untrusted header
field?

Earlier in rds_ib_process_recv(), the receiver extracts hdr->h_len from the
network and initializes ic->i_recv_data_rem:

net/rds/ib_recv.c:rds_ib_process_recv() {
    ...
    memcpy(hdr, ihdr, sizeof(*hdr));
    ic->i_recv_data_rem = be32_to_cpu(hdr->h_len);
    ...
}

It doesn't appear to validate it against RDS_MAX_MSG_SIZE. Can a malicious
remote peer stream data with a massive h_len to consume gigabytes of memory
per message and cause system exhaustion (OOM), especially since the same
unbounded h_len behavior appears to exist in TCP receive via
rds_tcp_data_recv()?

net/rds/tcp_recv.c:rds_tcp_data_recv() {
    ...
    if (tc->t_tinc_hdr_rem == 0) {
        /* could be 0 for a 0 len message */
        tc->t_tinc_data_rem =
            be32_to_cpu(tinc->ti_inc.i_hdr.h_len);
    ...
}

> +	/* The device wrote only data_len bytes into this recycled fragment
> +	 * page.  Zero the unwritten tail so a later rds_ib_inc_copy_to_user(),
> +	 * which copies up to the peer-declared h_len, cannot hand stale page
> +	 * contents to userspace.
> +	 */
> +	if (data_len < RDS_FRAG_SIZE) {
> +		void *frag = kmap_local_page(sg_page(&recv->r_frag->f_sg));
> +
> +		memset(frag + recv->r_frag->f_sg.offset + data_len, 0,
> +		       RDS_FRAG_SIZE - data_len);

[Severity: High]
Can zeroing the unwritten tail here cause silent data corruption and
desynchronize the protocol framing?

The commit message explicitly states that a zero-copy send with a small iovec
segment can legitimately produce a non-final fragment shorter than
RDS_FRAG_SIZE. However, the receiver's framing logic statically assumes every
non-final fragment contains exactly RDS_FRAG_SIZE bytes:

net/rds/ib_recv.c:rds_ib_process_recv() {
    ...
    if (ic->i_recv_data_rem > RDS_FRAG_SIZE)
        ic->i_recv_data_rem -= RDS_FRAG_SIZE;
    ...
}

And the copy routine blindly copies up to RDS_FRAG_SIZE bytes from each
fragment without checking the actual received data_len:

net/rds/ib_recv.c:rds_ib_inc_copy_to_user() {
    ...
    to_copy = min_t(unsigned long, iov_iter_count(to),
                    RDS_FRAG_SIZE - frag_off);
    to_copy = min_t(unsigned long, to_copy, len - copied);
    ...
}

Doesn't this mean the zeroes injected by the patch are copied to userspace as
valid payload bytes, shifting and truncating the actual data instead of
correctly handling short non-final fragments?

> +		kunmap_local(frag);
> +	}
> +
>  	list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
>  	recv->r_frag = NULL;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008124940.13371-1-shubham@octane.security?part=1

Re: [PATCH net v3] net/rds: ib: zero the unwritten tail of a short receive fragment

[Allison Henderson]

On Thu, 2026-10-08 at 18:19 +0530, Shubham Antil wrote:

Hi Shubham,

Thanks for the v3.  Some comments below:
 
> rds_ib_process_recv() accepts an RDS/IB fragment once the receive
> completion reports at least an RDS header, and then trusts the
> header-declared message length h_len: rds_ib_inc_copy_to_user() later
> copies up to h_len bytes from the fragment pages to userspace on
> recvmsg().
> 
> When a fragment carries fewer payload bytes than h_len accounts for, the
> bytes between the end of the received payload and h_len are read from the
> unwritten tail of the fragment page.  That page comes from the per-CPU
> receive cache and is not zeroed, so recvmsg() returns stale page contents
> to userspace.
> 
> Zero the unwritten tail of a short fragment page on receipt, so the copy
> can only ever return received payload or zeros.  Fragments that fill the
> page are unaffected.
> 
> Confirmed under KMSAN: without this change recvmsg() returns stale
> frag-page bytes and KMSAN flags an uninitialised copy in
> rds_ib_inc_copy_to_user(); with it the same over-read returns zeros and
> KMSAN is clean.
> 
> Fixes: 1e23b3ee0e94 ("RDS/IB: Receive datagrams via IB")
> Reported-by: sungbyeongchan <tjdqudcks0424@naver.com>
> Closes: https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/
> Reported-by: Shubham Antil <shubham@octane.security>
nit: since youre the author, you wouldn't add yourself as the reporter, so this tag should be trimmed out.

> Reported-by: Giovanni Vignone <gio@octane.security>
> Reported-by: Robert van Eijk <robert@octane.security>
> Reported-by: Paolo Gentry <paolo@octane.security>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Shubham Antil <shubham@octane.security>
> ---
> v3:
>  - Zero the unwritten fragment tail instead of dropping the connection.
>    A zero-copy send with a small iovec segment can legitimately produce a
>    non-final fragment shorter than RDS_FRAG_SIZE, so the v2 length check
>    could force a reconnect on well-formed traffic; zeroing the tail avoids
>    that while still keeping uninitialised bytes out of recvmsg().
rds_sendmsg() rejects MSG_ZEROCOPY for any transport other than TCP, so I dont
think this can happen over IB.

>    (Raised by the Sashiko review.)
I wasnt able to find the review that raised this, but the review for v2 landed after v3
was sent.  From your change log, it sounds like it was a different review, but the v2
findings are still valid, so those should be addressed:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007062135.49946-1-shubham%40octane.security

>  - Drop Reviewed-by, since the approach changed.
> v2:
>  - Add Reported-by (sungbyeongchan, first to report, and the Octane
>    Security reporters).
>  net/rds/ib_recv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> index bd6cb3ffa..9695c7f24 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c
> @@ -35,6 +35,7 @@
>  #include <linux/slab.h>
>  #include <linux/pci.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/highmem.h>
>  #include <rdma/rdma_cm.h>
>  
>  #include "rds_single_path.h"
> @@ -949,6 +950,19 @@ static void rds_ib_process_recv(struct rds_connection *conn,
>  		}
>  	}
>  
> +	/* The device wrote only data_len bytes into this recycled fragment
> +	 * page.  Zero the unwritten tail so a later rds_ib_inc_copy_to_user(),
> +	 * which copies up to the peer-declared h_len, cannot hand stale page
> +	 * contents to userspace.
> +	 */
> +	if (data_len < RDS_FRAG_SIZE) {
> +		void *frag = kmap_local_page(sg_page(&recv->r_frag->f_sg));
> +
> +		memset(frag + recv->r_frag->f_sg.offset + data_len, 0,
> +		       RDS_FRAG_SIZE - data_len);
I think padding with zeros would deliver a malformed message to the application as
if it were valid. A short fragment is a protocol violation, and resetting the connection
is what we already do for a fragment header mismatch.

I notice the dashboard says a review is still queued, so I'd recommend waiting for the
v3 review and then address both in a v4.

Thanks for working on this!
Allison

> +		kunmap_local(frag);
> +	}
> +
>  	list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
>  	recv->r_frag = NULL;
>  

Re: [PATCH net v3] net/rds: ib: zero the unwritten tail of a short receive fragment

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · 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] The patch closes the info leak, but its v3 approach rests on a
  false premise, and the commit message leaves out what the new memset…

--

net/rds: ib: zero the unwritten tail of a short receive fragment

This patch zeroes the unwritten tail of an RDS/IB receive fragment page in
rds_ib_process_recv() when the received payload is shorter than
RDS_FRAG_SIZE. The goal is to stop rds_ib_inc_copy_to_user() from copying
stale page contents to userspace when a peer sends a fragment shorter than
its h_len implies.

Closes: https://lore.kernel.org/netdev/20261006205204.1322102-1-tjdqudcks0424@naver.com/

> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> index bd6cb3ffaa571..9695c7f24eaa7 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c

[ ... ]

> @@ -949,6 +950,19 @@ static void rds_ib_process_recv(struct rds_connection *conn,
>  		}
>  	}
>  
> +	/* The device wrote only data_len bytes into this recycled fragment
> +	 * page.  Zero the unwritten tail so a later rds_ib_inc_copy_to_user(),
> +	 * which copies up to the peer-declared h_len, cannot hand stale page
> +	 * contents to userspace.
> +	 */
> +	if (data_len < RDS_FRAG_SIZE) {
> +		void *frag = kmap_local_page(sg_page(&recv->r_frag->f_sg));
> +
> +		memset(frag + recv->r_frag->f_sg.offset + data_len, 0,
> +		       RDS_FRAG_SIZE - data_len);
> +		kunmap_local(frag);
> +	}

[Severity: Medium]
Isn't this condition true for the final fragment of almost every well-formed
message? A message whose h_len is not a multiple of RDS_FRAG_SIZE always
ends with a short fragment. So a 100 byte message now pays for a kmap and a
memset of roughly 4KB in the receive completion path.

For well-formed traffic that work doesn't look necessary, because
rds_ib_inc_copy_to_user() already stops at h_len:

net/rds/ib_recv.c:rds_ib_inc_copy_to_user() {
    ...
		to_copy = min_t(unsigned long, to_copy, len - copied);
    ...
}

The stale tail can only be reached when a peer sends a short non-final
fragment, meaning data_len < min(RDS_FRAG_SIZE, ic->i_recv_data_rem).

The v3 changelog gives this reason for dropping the v2 length check:

  A zero-copy send with a small iovec segment can legitimately produce a
  non-final fragment shorter than RDS_FRAG_SIZE

Can that happen on RDS/IB? rds_sendmsg() rejects zero-copy on every
transport except TCP:

net/rds/send.c:rds_sendmsg() {
    ...
	if (zcopy) {
		if (rs->rs_transport->t_type != RDS_TRANS_TCP) {
			ret = -EOPNOTSUPP;
			goto out;
		}
    ...
}

On the copy path, rds_message_copy_from_user() gets its sg entries from
rds_page_remainder_alloc(). Every entry it returns is a full PAGE_SIZE
except the last one. rds_ib_xmit() then splits each entry with:

	len = min(RDS_FRAG_SIZE, sg_dma_len(scat) - rm->data.op_dmaoff);

That means only the last fragment of a message should ever be short.

Would it be better to bound the memset by
min(RDS_FRAG_SIZE, ic->i_recv_data_rem), which is already set at this
point? Rejecting that case as v2 did is another option. Either one should
still close the leak and leave well-formed traffic alone.

As written, a peer that sends short non-final fragments also gets its
message passed to the application with zero-filled holes, as if it were
valid.

The commit message also says:

  Fragments that fill the page are unaffected.

In the common small-message case the final fragment doesn't fill the page.
Could the changelog say that the memset runs on most messages?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008124940.13371-1-shubham%40octane.security