[PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length

[PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length

[Shubham Antil]

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

Re: [PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length

[sashiko-bot]

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

Re: [PATCH net v2] net/rds: ib: drop fragments shorter than the header-declared length

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] The patch is incomplete.

--

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

This patch adds a check to rds_ib_process_recv(). A fragment is rejected
when the payload length from the receive completion (data_len) is smaller
than the amount the fragment is accounted to contribute,
min(i_recv_data_rem, RDS_FRAG_SIZE). The goal is to stop
rds_ib_inc_copy_to_user() from copying uninitialized fragment page
contents 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)) {

[Severity: High]
Is data_len always the number of bytes written into the fragment page?
The commit message says:

  "The number of payload bytes actually received into the fragment page is
  data_len (after subtracting the header)"

rds_ib_recv_cqe_handler() checks only the completion status. It never
checks wc->opcode:

net/rds/ib_recv.c:rds_ib_recv_cqe_handler() {
    ...
	if (wc->status == IB_WC_SUCCESS) {
		rds_ib_process_recv(conn, recv, wc->byte_len, state);
    ...
}

An RDMA WRITE-with-immediate from the peer uses up a posted receive WR and
completes as IB_WC_RECV_RDMA_WITH_IMM. Its payload goes to the MR named by
the rkey, so nothing is written to the WR's header SGE or fragment SGE.
rxe still reports byte_len as the remote write length for this opcode:

drivers/infiniband/sw/rxe/rxe_resp.c:do_complete() {
    ...
	wc->byte_len = (pkt->mask & RXE_IMMDT_MASK &&
				pkt->mask & RXE_WRITE_MASK) ?
					qp->resp.length : wqe->dma.length - wqe->dma.resid;
    ...
}

RDS RC QPs get IB_ACCESS_REMOTE_WRITE through cm_init_qp_init_attr().
RDS MRs are registered with remote write access in the shared PD, so a
peer can get a writable rkey through the normal RDMA cookie exchange.

If the peer sends a WRITE_WITH_IMM of at least sizeof(struct rds_header)
+ RDS_FRAG_SIZE bytes, it looks like this happens:

  - The slot's i_recv_hdrs[] entry still holds the header from that
    slot's last use. That header has a valid checksum.

  - If no message is being reassembled, the old header starts a new
    message with its old h_len.

  - If a message is being reassembled, the old header can also pass the
    h_sequence, h_len and port mismatch checks.

  - data_len is large, so the new min_t() check passes.

  - The fragment page was never written. It is linked onto ii_frags
    anyway.

The page comes from the frag cache or rds_page_remainder_alloc() and is
not zeroed. rds_ib_inc_copy_to_user() would then copy up to h_len bytes
of it, which could include earlier RDS payloads meant for other sockets:

net/rds/ib_recv.c:rds_ib_inc_copy_to_user() {
    ...
		ret = copy_page_to_iter(sg_page(&frag->f_sg),
					frag->f_sg.offset + frag_off,
					to_copy,
					to);
    ...
}

Should completions with wc->opcode != IB_WC_RECV be rejected before
byte_len is trusted as the received length? The disclosure this patch
targets seems to remain reachable through this path.

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

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