Re: [PATCH net v4 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg()

[Dust Li <dust.li@linux.alibaba.com>]

On 2026-07-05 02:54:06, Bryam Vargas via B4 Relay wrote:
>From: Bryam Vargas <hexlabsecurity@proton.me>
>
>conn->bytes_to_rcv is accumulated in the receive tasklet from the
>peer's wire-controlled producer cursor via smc_curs_diff(), whose
>differing-wrap branch can exceed rmb_desc->len; a forged cursor drives
>bytes_to_rcv past the RMB, and over many CDC messages overflows the
>signed counter negative. smc_rx_recvmsg() reads it as the readable
>length and does a wrap-around copy whose second chunk is not re-bounded
>to rmb_desc->len, reading past the RMB into adjacent kernel memory and
>disclosing it to the peer. The nearby readable >= rmb_desc->len test
>only feeds SMC_STAT_RMB_RX_FULL on a separate earlier read; it does not
>bound the copy.
>
>Bound the readable length to rmb_desc->len at the consumer, treating a
>negative (sign-overflowed) value as out of range too, so the copy can
>never exceed the ring. This enforces the documented
>0 <= bytes_to_rcv <= rmb_desc->len invariant where it is race-free
>against the producer update in the tasklet; conforming peers are
>unaffected.
>
>Fixes: 952310ccf2d8 ("smc: receive data from RMBE")
>Cc: stable@vger.kernel.org
>Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>

Reviewed-by: Dust Li <dust.li@linux.alibaba.com>

Best regards,
Dust

>---
> net/smc/smc_rx.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
>index c1d9b923938d..f461cf10b085 100644
>--- a/net/smc/smc_rx.c
>+++ b/net/smc/smc_rx.c
>@@ -442,6 +442,18 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg,
> 		/* initialize variables for 1st iteration of subsequent loop */
> 		/* could be just 1 byte, even after waiting on data above */
> 		readable = smc_rx_data_available(conn, peeked_bytes);
>+		/* bytes_to_rcv is accumulated from the peer's wire-controlled
>+		 * producer cursor; a forged cursor can drive it past the RMB,
>+		 * or overflow the signed accumulator to a negative value across
>+		 * many CDC messages (which a plain "> len" check would miss
>+		 * before the size_t cast below turns it huge).  Bound it to the
>+		 * RMB in either case so the wrap-around copy cannot run past
>+		 * rmb_desc->len.  This enforces the documented
>+		 * 0 <= bytes_to_rcv <= rmb_desc->len invariant at the consumer,
>+		 * race-free against the producer update in the receive tasklet.
>+		 */
>+		if (readable < 0 || readable > conn->rmb_desc->len)
>+			readable = conn->rmb_desc->len;
> 		splbytes = atomic_read(&conn->splice_pending);
> 		if (!readable || (msg && splbytes)) {
> 			if (splbytes)
>
>-- 
>2.43.0
>