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

[sashiko-bot@kernel.org]

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

New issues:
- [High] Clamping a negative `bytes_to_rcv` to a positive length causes persistent phantom data delivery and infinite loops.

Pre-existing issues:
- [Critical] Unbounded `peeked_bytes` accumulation bypasses cursor wrap logic, leading to massive out-of-bounds read and kernel memory disclosure.
--

commit 7335ccb72e218a6f34e62494f13d66e4b8827bb3
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/smc: bound the receive length to the RMB in smc_rx_recvmsg()

The patch clamps the readable bytes calculation in smc_rx_recvmsg() to the RMB
length. This aims to prevent out-of-bounds reads if a forged producer cursor
drives bytes_to_rcv past the RMB size or into negative values.

> diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
> index c1d9b923938da..f461cf10b0855 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;

[Severity: Critical]
This isn't a bug introduced by this patch, but does unbounded peeked_bytes
accumulation bypass the cursor wrap logic?

If bytes_to_rcv is negative, subtracting peeked_bytes in smc_rx_data_available()
guarantees readable is negative. Clamping this negative value to a positive
buffer length (conn->rmb_desc->len) allows the loop to iterate indefinitely,
growing peeked_bytes unboundedly.

When peeked_bytes exceeds 2 * conn->rmb_desc->len, is it correctly wrapped
when applied to the cursor?

net/smc/smc_cdc.h:smc_curs_add() {
    curs->count += value;
    if (curs->count >= size) {
        curs->wrap++;
        curs->count -= size;
    }
}

Since smc_curs_add() only subtracts size once, cons.count will remain >= size
if value is massive.

This out-of-bounds cursor would then cause an underflow in the chunk length
calculation:

net/smc/smc_rx.c:smc_rx_recvmsg() {
        ...
		/* determine chunks where to read from rcvbuf */
		/* either unwrapped case, or 1st chunk of wrapped case */
		chunk_len = min_t(size_t, copylen, conn->rmb_desc->len -
				  cons.count);
        ...
}

Could this underflow result in a massive chunk_len and a subsequent
out-of-bounds kernel memory read?

[Severity: High]
Will clamping a negative bytes_to_rcv to a positive length cause an infinite
stream of phantom data?

If readable is clamped to a positive value when bytes_to_rcv is negative, the
function will authorize the phantom read and copy rmb_desc->len bytes of
stale data.

At the end of the loop:

net/smc/smc_rx.c:smc_rx_recvmsg() {
        ...
		/* update cursors */
		if (!(flags & MSG_PEEK)) {
			/* increased in recv tasklet smc_cdc_msg_rcv() */
			smp_mb__before_atomic();
			atomic_sub(copylen, &conn->bytes_to_rcv);
        ...
}

When atomic_sub() executes, subtracting copylen from the already negative
bytes_to_rcv makes it even more negative.

Since bytes_to_rcv is never zeroed or corrected, will the next recvmsg()
call see a negative value, clamp it to rmb_desc->len again, and deliver another
chunk of phantom data indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614-b4-disp-edd64be9-v3-0-551fa514257e@proton.me?part=2