[PATCH] net/smc: bound sndbuf_space on the SMC-D DMB-merge receive path

[PATCH] net/smc: bound sndbuf_space on the SMC-D DMB-merge receive path

[Bryam Vargas]

When the SMC-D send buffer is merged with the peer's DMB (the nocopy
DMB-merge path), smc_cdc_msg_recv_action() advances the local send-buffer
free space from the peer's consumer cursor:

	diff_tx = smc_curs_diff(conn->sndbuf_desc->len,
				&conn->tx_curs_fin,
				&conn->local_rx_ctrl.cons);
	atomic_add(diff_tx, &conn->sndbuf_space);

conn->local_rx_ctrl.cons is the peer's consumer cursor, copied verbatim
from the wire by smcd_cdc_msg_to_host() with no validation.
smc_curs_diff()'s differing-wrap branch returns
(size - old.count) + new.count, which exceeds size for a forged cursor,
so a malicious peer can drive sndbuf_space past sndbuf_desc->len. The
"guarantee 0 <= sndbuf_space <= sndbuf_desc->len" comment on the
atomic_add() is not enforced.

smc_tx_sendmsg() then reads the inflated sndbuf_space as the available
write space and copies that many user bytes into the send buffer; its
two-chunk wrap-around copy bounds only the first chunk to
sndbuf_desc->len, so the second chunk (copylen - first_chunk, written at
offset 0) runs past the send buffer -- a heap out-of-bounds write of
attacker-influenced length with user-controlled content.

Enforce the documented invariant after the cursor-driven atomic_add(), as
the SMC-D receive path already does for bytes_to_rcv.

Fixes: cc0ab806fc52 ("net/smc: adapt cursor update when sndbuf and peer DMB are merged")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
The out-of-bounds write was reproduced under KASAN by driving
smc_tx_sendmsg()'s two-chunk send-ring copy with an inflated sndbuf_space
(slab-out-of-bounds Write); with the clamp the same input keeps the copy
within sndbuf_desc->len. The DMB-merge/nocopy path that lets a peer
consumer cursor inflate sndbuf_space is reachable with the in-kernel
loopback-ism (CONFIG_SMC_LO), which advertises dmb_nocopy, on commodity
x86 -- no special hardware is required to exercise it.

 net/smc/smc_cdc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index 619b3bab3824..cf8d65407ea5 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -365,6 +365,10 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
 			smp_mb__before_atomic();
 			atomic_add(diff_tx, &conn->sndbuf_space);
 			/* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
+			if (atomic_read(&conn->sndbuf_space) >
+			    conn->sndbuf_desc->len)
+				atomic_set(&conn->sndbuf_space,
+					   conn->sndbuf_desc->len);
 			smp_mb__after_atomic();
 			smc_curs_copy(&conn->tx_curs_fin,
 				      &conn->local_rx_ctrl.cons, conn);
-- 
2.43.0

Re: [PATCH] net/smc: bound sndbuf_space on the SMC-D DMB-merge receive path

[sashiko-bot]

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

New issues:
- [High] TOCTOU race condition in `sndbuf_space` clamp allows heap out-of-bounds write

Pre-existing issues:
- [Critical] Unchecked `bytes_to_rcv` inflation leading to heap out-of-bounds read
- [Critical] Signed integer overflow bypasses the `sndbuf_space` clamp check
--

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

net/smc: bound sndbuf_space on the SMC-D DMB-merge receive path

This commit modifies the receive path to clamp sndbuf_space after calculating
the cursor difference. It uses an atomic_add followed by a conditional
atomic_set to enforce the bounds. The intent is to prevent sndbuf_space from
exceeding the send buffer length.

> Enforce the documented invariant after the cursor-driven atomic_add(), as
> the SMC-D receive path already does for bytes_to_rcv.

[Severity: Critical]
This is a pre-existing issue, but does the receive path actually enforce
invariants for bytes_to_rcv as claimed here?

In smc_cdc_msg_recv_action(), diff_prod is calculated from an unvalidated
peer cursor and atomically added to conn->bytes_to_rcv without any clamping:

net/smc/smc_cdc.c:smc_cdc_msg_recv_action() {
    ...
    atomic_add(diff_prod, &conn->bytes_to_rcv);
    /* guarantee 0 <= bytes_to_rcv <= rmb_desc->len */
    smp_mb__after_atomic();
    ...
}

If the cdc->prod cursor causes an inflated bytes_to_rcv value, and
smc_rx_recvmsg() reads this value:

net/smc/smc_rx.c:smc_rx_recvmsg() {
    ...
    copylen = min_t(size_t, read_remaining, readable);
    ...
}

Could the length of the second chunk exceed the buffer size, causing
memcpy_to_msg() or splice_to_pipe() to read memory out of bounds?

> diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
> index 619b3bab38248..cf8d65407ea5d 100644
> --- a/net/smc/smc_cdc.c
> +++ b/net/smc/smc_cdc.c
> @@ -365,6 +365,10 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
>  			smp_mb__before_atomic();
>  			atomic_add(diff_tx, &conn->sndbuf_space);
>  			/* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
> +			if (atomic_read(&conn->sndbuf_space) >
> +			    conn->sndbuf_desc->len)
> +				atomic_set(&conn->sndbuf_space,
> +					   conn->sndbuf_desc->len);

[Severity: High]
Can this non-atomic clamping sequence create a race condition?

If smc_tx_sendmsg() runs concurrently in process context, could it locklessly
read the temporarily inflated sndbuf_space between the atomic_add() and
atomic_set() calls?

net/smc/smc_tx.c:smc_tx_sendmsg() {
    ...
    writespace = atomic_read(&conn->sndbuf_space);
    ...
}

Using this transiently inflated value, could the second loop iteration in
smc_tx_sendmsg() write memory out of bounds past the send buffer?

[Severity: Critical]
This is a pre-existing issue, but does this signed comparison fail to catch
negative wrapped values caused by signed integer overflow?

If a forged cursor causes smc_curs_diff() to return a very large value,
and this is added via atomic_add(), the 32-bit signed integer could overflow
into a negative value. The check would then compare this negative value
against the positive sndbuf_desc->len, evaluate to false, and bypass the
clamp entirely.

In smc_tx_sendmsg(), if this negative writespace is cast to size_t:

net/smc/smc_tx.c:smc_tx_sendmsg() {
    ...
    copylen = min_t(size_t, send_remaining, writespace);
    ...
}

Would this result in an unexpectedly large unsigned value, causing the
second loop iteration to write memory out of bounds?

>  			smp_mb__after_atomic();
>  			smc_curs_copy(&conn->tx_curs_fin,
>  				      &conn->local_rx_ctrl.cons, conn);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610090928.192177-1-hexlabsecurity@proton.me?part=1

Re: [PATCH] net/smc: bound sndbuf_space on the SMC-D DMB-merge receive path

[Jakub Kicinski]

On Wed, 10 Jun 2026 09:09:36 +0000 Bryam Vargas wrote:
>  			/* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
> +			if (atomic_read(&conn->sndbuf_space) >
> +			    conn->sndbuf_desc->len)
> +				atomic_set(&conn->sndbuf_space,
> +					   conn->sndbuf_desc->len);

similar comments here as on the previous patch
-- 
pw-bot: cr