Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length
@ 2026-10-07  6:21 Shubham Antil
  2026-10-08  6:21 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Shubham Antil @ 2026-10-07  6:21 UTC (permalink / raw)
  To: netdev, linux-rdma
  Cc: Allison Henderson, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, linux-kernel, Giovanni Vignone,
	sungbyeongchan

rds_ib_process_recv() accepts an incoming RDS/IB fragment once the
receive completion reports at least an RDS header
(data_len >= sizeof(struct rds_header)).  It then trusts the
header-declared total message length h_len: for the first fragment of
a message it stores be32_to_cpu(hdr->h_len) in ic->i_recv_data_rem,
and rds_ib_inc_copy_to_user() later copies up to h_len bytes from the
fragment pages to userspace on recvmsg().

The number of payload bytes actually received into the fragment page is
data_len (after subtracting the header), but it is never checked against
the amount the fragment is accounted to contribute to the message,
min(i_recv_data_rem, RDS_FRAG_SIZE).  A fragment whose header advertises
a larger h_len than the payload it delivers is still linked onto the
reassembly list.  The fragment page comes from the per-CPU receive cache
and is not zeroed, so rds_ib_inc_copy_to_user() then copies up to h_len
bytes to the PF_RDS reader, including the uninitialized tail the receive
never wrote.

Reject a fragment that carries fewer payload bytes than it is accounted
to contribute before linking it onto the reassembly list.

The issue is reproducible under KMSAN with two hosts over rdma_rxe
(Soft-RoCE); the same reproducer confirms the fix stops it.

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>
Reviewed-by: Allison Henderson <achender@kernel.org>
---
v2:
 - No functional change.
 - Add Reported-by for sungbyeongchan (first to report; see Closes link)
   and for the Octane Security reporters.
 - Add Allison's Reviewed-by.
 net/rds/ib_recv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index bd6cb3ffa..fee77b6d7 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -949,6 +949,15 @@ static void rds_ib_process_recv(struct rds_connection *conn,
 		}
 	}
 
+	/* h_len must be backed by the payload actually received (data_len),
+	 * else the unwritten frag-page tail is copied to userspace.
+	 */
+	if (data_len < min_t(u32, ic->i_recv_data_rem, RDS_FRAG_SIZE)) {
+		rds_ib_conn_error(conn,
+			"fragment shorter than header-declared length; forcing reconnect\n");
+		goto done;
+	}
+
 	list_add_tail(&recv->r_frag->f_item, &ibinc->ii_frags);
 	recv->r_frag = NULL;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length
  2026-10-07  6:21 [PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length Shubham Antil
@ 2026-10-08  6:21 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-08  6:21 UTC (permalink / raw)
  To: Shubham Antil; +Cc: linux-rdma

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rds: Denial of Service (connection drop) via fragmented ZCOPY messages
--

commit 99e06641e00267bc78ccb17feb870bd7545f3c4f
Author: Shubham Antil <shubham@octane.security>

net/rds: ib: drop fragments shorter than the header-declared length

This commit introduces a strict length check in rds_ib_process_recv() to drop
incoming fragments that are shorter than the header-declared length. This is
intended to prevent uninitialized memory in the fragment page from being copied
to userspace.

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..fee77b6d7ffb2 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c
> @@ -949,6 +949,15 @@ static void rds_ib_process_recv(struct rds_connection *conn,
>  		}
>  	}
>  
> +	/* h_len must be backed by the payload actually received (data_len),
> +	 * else the unwritten frag-page tail is copied to userspace.
> +	 */
> +	if (data_len < min_t(u32, ic->i_recv_data_rem, RDS_FRAG_SIZE)) {
> +		rds_ib_conn_error(conn,
> +			"fragment shorter than header-declared length; forcing reconnect\n");
> +		goto done;
> +	}
> +

[Severity: High]
Does this new check assume all non-final fragments are perfectly sized to
RDS_FRAG_SIZE?

When a user calls sendmsg() on an RDS socket with zero-copy enabled, passing
an iovec array where intermediate elements are smaller than RDS_FRAG_SIZE,
rds_message_zcopy_from_user() preserves these small user buffers by creating
small SG elements.

Looking at rds_ib_xmit() in net/rds/ib_send.c, the sender does not coalesce
small SG elements across RDS_FRAG_SIZE boundaries:

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

	send->s_sge[1].addr = sg_dma_address(scat);
	send->s_sge[1].addr += rm->data.op_dmaoff;
	send->s_sge[1].length = len;
	send->s_sge[1].lkey = ic->i_pd->local_dma_lkey;

	bytes_sent += len;
	rm->data.op_dmaoff += len;
	if (rm->data.op_dmaoff == sg_dma_len(scat)) {
		scat++;

Because rds_ib_xmit() directly creates an IB SEND fragment bounded by the
small SG element length, the received fragment might legitimately be smaller
than RDS_FRAG_SIZE even if it is not the final fragment of the message.

Could this allow any local user with RDS access to trivially tear down active
RDS IB connections by sending a valid zero-copy message constructed with small
iovec lengths, causing rds_ib_conn_error() to be hit on the receiver?

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

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-08  6:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07  6:21 [PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length Shubham Antil
2026-10-08  6:21 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox