[PATCH net v5 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers

[PATCH net v5 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers

[Bryam Vargas]

A peer's CDC producer/consumer cursors are copied from the wire and used,
without an upper bound against the local buffers, as (a) a raw index into the
RMB on the urgent path, (b) the receive length in smc_rx_recvmsg(), and (c) the
send length in smc_tx_sendmsg() on the SMC-D DMB-merge path. A malicious or
buggy peer can forge a cursor so each runs past the relevant buffer: an
out-of-bounds read of adjacent kernel memory (disclosed to the peer) on the
receive/urgent side, and, on the send side, an out-of-bounds write whose
length the peer controls and whose overflowing bytes are the local sender's
own outbound data.

This series bounds each length where it is consumed. The clamp is synchronous
and race-free against the tasklet that advances the cursor, so it is the minimal
fix for stable. A separate net-next series adds the wire-boundary validation and
connection abort that Dust Li suggested; those do not replace these clamps.

The clamp is not subsumed by validating cursors at the input boundary. A peer
that only increments prod.wrap with count == 0 hits the differing-wrap branch of
smc_curs_diff(), which returns (len - 0) + 0 == len every CDC, so bytes_to_rcv
(and sndbuf_space on the send side) accumulates past the buffer while every
per-cursor bound sees count == 0 and accepts the message. The overflow lives in
the accumulator, not the cursor; only the consumer-side clamp bounds it. And
because a queued abort runs asynchronously (queue_work -> smc_conn_kill) while
smc_rx_recvmsg() reads the accumulator under lock_sock, only the synchronous
clamp closes that window. So the clamp goes to stable; the abort is net-next.

The nearby readable >= rmb_desc->len / len > sndbuf_desc->len tests only feed
statistics counters (SMC_STAT_RMB_RX_FULL / SMC_STAT_RMB_TX_SIZE_SMALL) on an
earlier, separate read; they do not bound the copy.

A/B (in-kernel KASAN replaying the sink arithmetic over a real rmb_desc->len /
sndbuf_desc->len slab; kasan.fault=report kasan_multi_shot, 2026-07-05):
  - urgent index (1/3):  count = len+1        -> slab-out-of-bounds Read;  clamped -> clean
  - recv length  (2/3):  bytes_to_rcv = 5*len via wrap++/count=0 -> OOB Read; clamped -> clean
  - send length  (3/3):  sndbuf_space inflated -> slab-out-of-bounds Write; clamped -> clean
  - signed overflow:     readable = -1 -> v1 ">len" misses -> OOB; "<0 || >len" -> clean
  - concurrent TOCTOU race: a producer-side clamp is racy (OOB in a racing consumer
    kthread on another CPU); the consumer-side clamp is race-free (0 hits / 5,000,000 reads).
  - every in-bounds / honest-peer arm: clean.

Changes since v4:
  - repost only: the netdev patch queue overflowed, so this reposts the v4
    series unchanged, carrying Dust Li's Reviewed-by on all three patches.
  v4: https://lore.kernel.org/all/20260705-b4-disp-28a1bbca-v4-0-be089b98acc6@proton.me/

Changes since v3:
  - split into this stable-bound clamp series and a separate net-next
    validate/abort series, per Dust Li's review;
  - tightened the commit messages; noted that the nearby SMC_STAT_* tests are not
    bounds; no functional change to the three clamps.
  v3: https://lore.kernel.org/all/20260614-b4-disp-edd64be9-v3-0-551fa514257e@proton.me/

Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Bryam Vargas (3):
      net/smc: bound the wire-controlled producer cursor to the RMB
      net/smc: bound the receive length to the RMB in smc_rx_recvmsg()
      net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

 net/smc/smc_cdc.h | 27 ++++++++++++++++++++++++---
 net/smc/smc_rx.c  | 12 ++++++++++++
 net/smc/smc_tx.c  | 13 +++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)
---
base-commit: 4539944e515183668109bdf4d0c3d7d228383d88
change-id: 20260723-b4-disp-0d07164f-aa04045ccd98

Best regards,
--  
Bryam Vargas <hexlabsecurity@proton.me>

[PATCH net v5 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers

[Bryam Vargas via B4 Relay]

A peer's CDC producer/consumer cursors are copied from the wire and used,
without an upper bound against the local buffers, as (a) a raw index into the
RMB on the urgent path, (b) the receive length in smc_rx_recvmsg(), and (c) the
send length in smc_tx_sendmsg() on the SMC-D DMB-merge path. A malicious or
buggy peer can forge a cursor so each runs past the relevant buffer: an
out-of-bounds read of adjacent kernel memory (disclosed to the peer) on the
receive/urgent side, and, on the send side, an out-of-bounds write whose
length the peer controls and whose overflowing bytes are the local sender's
own outbound data.

This series bounds each length where it is consumed. The clamp is synchronous
and race-free against the tasklet that advances the cursor, so it is the minimal
fix for stable. A separate net-next series adds the wire-boundary validation and
connection abort that Dust Li suggested; those do not replace these clamps.

The clamp is not subsumed by validating cursors at the input boundary. A peer
that only increments prod.wrap with count == 0 hits the differing-wrap branch of
smc_curs_diff(), which returns (len - 0) + 0 == len every CDC, so bytes_to_rcv
(and sndbuf_space on the send side) accumulates past the buffer while every
per-cursor bound sees count == 0 and accepts the message. The overflow lives in
the accumulator, not the cursor; only the consumer-side clamp bounds it. And
because a queued abort runs asynchronously (queue_work -> smc_conn_kill) while
smc_rx_recvmsg() reads the accumulator under lock_sock, only the synchronous
clamp closes that window. So the clamp goes to stable; the abort is net-next.

The nearby readable >= rmb_desc->len / len > sndbuf_desc->len tests only feed
statistics counters (SMC_STAT_RMB_RX_FULL / SMC_STAT_RMB_TX_SIZE_SMALL) on an
earlier, separate read; they do not bound the copy.

A/B (in-kernel KASAN replaying the sink arithmetic over a real rmb_desc->len /
sndbuf_desc->len slab; kasan.fault=report kasan_multi_shot, 2026-07-05):
  - urgent index (1/3):  count = len+1        -> slab-out-of-bounds Read;  clamped -> clean
  - recv length  (2/3):  bytes_to_rcv = 5*len via wrap++/count=0 -> OOB Read; clamped -> clean
  - send length  (3/3):  sndbuf_space inflated -> slab-out-of-bounds Write; clamped -> clean
  - signed overflow:     readable = -1 -> v1 ">len" misses -> OOB; "<0 || >len" -> clean
  - concurrent TOCTOU race: a producer-side clamp is racy (OOB in a racing consumer
    kthread on another CPU); the consumer-side clamp is race-free (0 hits / 5,000,000 reads).
  - every in-bounds / honest-peer arm: clean.

Changes since v4:
  - repost only: the netdev patch queue overflowed, so this reposts the v4
    series unchanged, carrying Dust Li's Reviewed-by on all three patches.
  v4: https://lore.kernel.org/all/20260705-b4-disp-28a1bbca-v4-0-be089b98acc6@proton.me/

Changes since v3:
  - split into this stable-bound clamp series and a separate net-next
    validate/abort series, per Dust Li's review;
  - tightened the commit messages; noted that the nearby SMC_STAT_* tests are not
    bounds; no functional change to the three clamps.
  v3: https://lore.kernel.org/all/20260614-b4-disp-edd64be9-v3-0-551fa514257e@proton.me/

Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Bryam Vargas (3):
      net/smc: bound the wire-controlled producer cursor to the RMB
      net/smc: bound the receive length to the RMB in smc_rx_recvmsg()
      net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

 net/smc/smc_cdc.h | 27 ++++++++++++++++++++++++---
 net/smc/smc_rx.c  | 12 ++++++++++++
 net/smc/smc_tx.c  | 13 +++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)
---
base-commit: 4539944e515183668109bdf4d0c3d7d228383d88
change-id: 20260723-b4-disp-0d07164f-aa04045ccd98

Best regards,
--  
Bryam Vargas <hexlabsecurity@proton.me>

[PATCH net v5 1/3] net/smc: bound the wire-controlled producer cursor to the RMB

[Bryam Vargas]

smcr_cdc_msg_to_host() and smcd_cdc_msg_to_host() import a peer's
producer cursor from the wire into conn->local_rx_ctrl.prod without
bounding it against the receive buffer. The urgent-data path in
smc_cdc_msg_recv_action() then uses that count as a raw index into the
RMB, so a peer that advertises a producer cursor past rmb_desc->len
reads out of bounds of the RMB allocation in the receive tasklet and
can disclose adjacent kernel memory.

Bound the producer cursor count to rmb_desc->len at the wire-to-host
conversion, for both SMC-R and SMC-D. Bound only the producer cursor:
the consumer cursor indexes the peer's RMB and is bounded by
peer_rmbe_size, so clamping it to our rmb_desc->len would under-credit
peer_rmbe_space and stall transmit to a peer with a larger RMB.
Conforming peers are unaffected.

Fixes: de8474eb9d50 ("net/smc: urgent data support")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
 net/smc/smc_cdc.h | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 696cc11f2303..ca76ef630356 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h
@@ -221,7 +221,8 @@ static inline void smc_host_msg_to_cdc(struct smc_cdc_msg *peer,
 
 static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
 					  union smc_cdc_cursor *peer,
-					  struct smc_connection *conn)
+					  struct smc_connection *conn,
+					  int max_count)
 {
 	union smc_host_cursor temp, old;
 	union smc_cdc_cursor net;
@@ -235,6 +236,15 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
 	if ((old.wrap == temp.wrap) &&
 	    (old.count > temp.count))
 		return;
+	/* The peer producer cursor is wire-controlled and is later used as a
+	 * raw index into our RMB by the urgent path; bound its count to the
+	 * RMB.  max_count == 0 leaves the consumer cursor unbounded here: it
+	 * indexes the peer's RMB (bounded by peer_rmbe_size, not our
+	 * rmb_desc->len), so clamping it to rmb_desc->len would under-credit
+	 * peer_rmbe_space and stall transmit to peers with a larger RMB.
+	 */
+	if (max_count && temp.count > max_count)
+		temp.count = max_count;
 	smc_curs_copy(local, &temp, conn);
 }
 
@@ -246,8 +256,13 @@ static inline void smcr_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 	local->len = peer->len;
 	local->seqno = ntohs(peer->seqno);
 	local->token = ntohl(peer->token);
-	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn);
-	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn);
+	/* bound the wire-controlled producer cursor to our RMB (used as a raw
+	 * index by the urgent path); leave the consumer cursor unbounded -- it
+	 * indexes the peer's RMB and is bounded by peer_rmbe_size.
+	 */
+	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn,
+			       conn->rmb_desc->len);
+	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn, 0);
 	local->prod_flags = peer->prod_flags;
 	local->conn_state_flags = peer->conn_state_flags;
 }
@@ -260,6 +275,12 @@ static inline void smcd_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 
 	temp.wrap = peer->prod.wrap;
 	temp.count = peer->prod.count;
+	/* the peer producer cursor is wire-controlled and is used as a raw
+	 * index into our RMB by the urgent path; bound it to the RMB.  The
+	 * consumer cursor below indexes the peer's RMB and is left unbounded.
+	 */
+	if (temp.count > conn->rmb_desc->len)
+		temp.count = conn->rmb_desc->len;
 	smc_curs_copy(&local->prod, &temp, conn);
 
 	temp.wrap = peer->cons.wrap;

-- 
2.55.0

[PATCH net v5 1/3] net/smc: bound the wire-controlled producer cursor to the RMB

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

smcr_cdc_msg_to_host() and smcd_cdc_msg_to_host() import a peer's
producer cursor from the wire into conn->local_rx_ctrl.prod without
bounding it against the receive buffer. The urgent-data path in
smc_cdc_msg_recv_action() then uses that count as a raw index into the
RMB, so a peer that advertises a producer cursor past rmb_desc->len
reads out of bounds of the RMB allocation in the receive tasklet and
can disclose adjacent kernel memory.

Bound the producer cursor count to rmb_desc->len at the wire-to-host
conversion, for both SMC-R and SMC-D. Bound only the producer cursor:
the consumer cursor indexes the peer's RMB and is bounded by
peer_rmbe_size, so clamping it to our rmb_desc->len would under-credit
peer_rmbe_space and stall transmit to a peer with a larger RMB.
Conforming peers are unaffected.

Fixes: de8474eb9d50 ("net/smc: urgent data support")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
 net/smc/smc_cdc.h | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 696cc11f2303..ca76ef630356 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h
@@ -221,7 +221,8 @@ static inline void smc_host_msg_to_cdc(struct smc_cdc_msg *peer,
 
 static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
 					  union smc_cdc_cursor *peer,
-					  struct smc_connection *conn)
+					  struct smc_connection *conn,
+					  int max_count)
 {
 	union smc_host_cursor temp, old;
 	union smc_cdc_cursor net;
@@ -235,6 +236,15 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
 	if ((old.wrap == temp.wrap) &&
 	    (old.count > temp.count))
 		return;
+	/* The peer producer cursor is wire-controlled and is later used as a
+	 * raw index into our RMB by the urgent path; bound its count to the
+	 * RMB.  max_count == 0 leaves the consumer cursor unbounded here: it
+	 * indexes the peer's RMB (bounded by peer_rmbe_size, not our
+	 * rmb_desc->len), so clamping it to rmb_desc->len would under-credit
+	 * peer_rmbe_space and stall transmit to peers with a larger RMB.
+	 */
+	if (max_count && temp.count > max_count)
+		temp.count = max_count;
 	smc_curs_copy(local, &temp, conn);
 }
 
@@ -246,8 +256,13 @@ static inline void smcr_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 	local->len = peer->len;
 	local->seqno = ntohs(peer->seqno);
 	local->token = ntohl(peer->token);
-	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn);
-	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn);
+	/* bound the wire-controlled producer cursor to our RMB (used as a raw
+	 * index by the urgent path); leave the consumer cursor unbounded -- it
+	 * indexes the peer's RMB and is bounded by peer_rmbe_size.
+	 */
+	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn,
+			       conn->rmb_desc->len);
+	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn, 0);
 	local->prod_flags = peer->prod_flags;
 	local->conn_state_flags = peer->conn_state_flags;
 }
@@ -260,6 +275,12 @@ static inline void smcd_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 
 	temp.wrap = peer->prod.wrap;
 	temp.count = peer->prod.count;
+	/* the peer producer cursor is wire-controlled and is used as a raw
+	 * index into our RMB by the urgent path; bound it to the RMB.  The
+	 * consumer cursor below indexes the peer's RMB and is left unbounded.
+	 */
+	if (temp.count > conn->rmb_desc->len)
+		temp.count = conn->rmb_desc->len;
 	smc_curs_copy(&local->prod, &temp, conn);
 
 	temp.wrap = peer->cons.wrap;

-- 
2.55.0

Re: [PATCH net v5 1/3] net/smc: bound the wire-controlled producer cursor to the RMB

[sashiko-bot]

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

New issues:
- [High] NULL pointer dereference by sending an early CDC message with a guessed connection token before the connection's receive buffer (`rmb_desc`) is allocated.

Pre-existing issues:
- [Critical] Heap Out-of-Bounds Write due to unbounded consumer cursor inflating `sndbuf_space`.
- [High] A malicious peer can continuously inflate `conn->bytes_to_rcv` beyond the receive buffer size by manipulating the `wrap` field (or incrementally advancing the `count` field) of the producer cursor, leading to a massive out-of-bounds read during `recvmsg`.
--

--- Patch [1]: [PATCH net v5 1/3] net/smc: bound the wire-controlled producer cursor to the RMB ---
commit bb6f7b4d6d71f8888e49e10514b07e9e257bfa1f
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/smc: bound the wire-controlled producer cursor to the RMB

This patch bounds the producer cursor count to the receive buffer length at
the wire-to-host conversion for both SMC-R and SMC-D. It leaves the consumer
cursor unbounded, as it indexes the peer's RMB and clamping it would stall
transmit to peers with larger RMBs.

> diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
> index 696cc11f2303b..ca76ef6303566 100644
> --- a/net/smc/smc_cdc.h
> +++ b/net/smc/smc_cdc.h

[ ... ]

> @@ -235,6 +236,15 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
>  	if ((old.wrap == temp.wrap) &&
>  	    (old.count > temp.count))
>  		return;
> +	/* The peer producer cursor is wire-controlled and is later used as a
> +	 * raw index into our RMB by the urgent path; bound its count to the
> +	 * RMB.  max_count == 0 leaves the consumer cursor unbounded here: it
> +	 * indexes the peer's RMB (bounded by peer_rmbe_size, not our
> +	 * rmb_desc->len), so clamping it to rmb_desc->len would under-credit
> +	 * peer_rmbe_space and stall transmit to peers with a larger RMB.
> +	 */
> +	if (max_count && temp.count > max_count)
> +		temp.count = max_count;

[Severity: High]
This is a pre-existing issue, but does bounding only the count field leave
the wrap field unchecked?

If a peer sends multiple CDC messages where wrap increments, or count
incrementally advances without exceeding rmb_desc->len, could smc_curs_diff()
compute a positive diff_prod?

net/smc/smc_cdc.c:smc_cdc_msg_recv_action() {
    ...
	diff_prod = smc_curs_diff(conn->rmb_desc->len, &prod_old,
				  &conn->local_rx_ctrl.prod);
	if (diff_prod) {
    ...
		atomic_add(diff_prod, &conn->bytes_to_rcv);
    ...
}

Could this unconditionally inflate conn->bytes_to_rcv beyond the receive
buffer size, causing copylen to exceed the buffer length in smc_rx_recvmsg()?

net/smc/smc_rx.c:smc_rx_recvmsg() {
    ...
			chunk_len = copylen - chunk_len;
			rc = memcpy_to_msg(msg, rcvbuf_base, chunk_len);
    ...
}

Could this result in an out-of-bounds read of adjacent kernel memory?

>  	smc_curs_copy(local, &temp, conn);
>  }
>  
> @@ -246,8 +256,13 @@ static inline void smcr_cdc_msg_to_host(struct smc_host_cdc_msg *local,
>  	local->len = peer->len;
>  	local->seqno = ntohs(peer->seqno);
>  	local->token = ntohl(peer->token);
> -	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn);
> -	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn);
> +	/* bound the wire-controlled producer cursor to our RMB (used as a raw
> +	 * index by the urgent path); leave the consumer cursor unbounded -- it
> +	 * indexes the peer's RMB and is bounded by peer_rmbe_size.
> +	 */
> +	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn,
> +			       conn->rmb_desc->len);

[Severity: High]
Can conn->rmb_desc be NULL here?

If a CDC message is received during connection setup before the receive
buffers are allocated:

net/smc/smc_cdc.c:smc_cdc_rx_handler() {
    ...
	conn = smc_lgr_find_conn(ntohl(cdc->token), lgr);
	if (!conn || conn->out_of_sync) {
		read_unlock_bh(&lgr->conns_lock);
		return;
	}
    ...
}

If the connection is found before smc_buf_create() completes, does
dereferencing conn->rmb_desc->len result in a NULL pointer dereference?

> +	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn, 0);

[Severity: Critical]
This is a pre-existing issue, but does leaving the consumer cursor unbounded
allow inflating conn->sndbuf_space?

If peer->cons.count is set to a very large value, could smc_curs_diff() return
a large diff_tx?

net/smc/smc_cdc.c:smc_cdc_msg_recv_action() {
    ...
			diff_tx = smc_curs_diff(conn->sndbuf_desc->len,
						&conn->tx_curs_fin,
						&conn->local_rx_ctrl.cons);
			/* increase local sndbuf space and fin_curs */
			smp_mb__before_atomic();
			atomic_add(diff_tx, &conn->sndbuf_space);
    ...
}

When sendmsg() is subsequently called, could this inflated sndbuf_space cause
copylen to be miscalculated?

net/smc/smc_tx.c:smc_tx_sendmsg() {
    ...
			chunk_len = copylen - chunk_len;
			rc = memcpy_from_msg(sndbuf_base, msg, chunk_len);
    ...
}

Could this lead to an out-of-bounds write from userspace into the
kernel heap?

>  	local->prod_flags = peer->prod_flags;
>  	local->conn_state_flags = peer->conn_state_flags;
>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-disp-0d07164f-v5-0-6a9e235dbc4e@proton.me?part=1

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

[Bryam Vargas]

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>
---
 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.55.0

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

[Bryam Vargas via B4 Relay]

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>
---
 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.55.0

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The patch fails to fully prevent out-of-bounds reads because uncorrected negative `bytes_to_rcv` allows `MSG_PEEK` accumulations to exceed `2 * rmb_desc->len`, triggering a wrapping logic bug in `smc_curs_add()`.
- [Critical] In `smc_rx_splice()`, socket and page references are acquired *after* `splice_to_pipe()` returns, creating a race condition (Use-After-Free) and a refcount underflow for multi-page splices.
- [High] The patch prevents an immediate out-of-bounds read but fails to correct the corrupted (negative) `bytes_to_rcv` state, leading to a persistent denial of service, infinite garbage data reads, and corrupted `FIONREAD` ioctl results.
- [High] If `memcpy_to_msg()` or `smc_rx_splice()` fails on the second chunk of a wrapped read, `smc_rx_recvmsg()` returns success for the first chunk but fails to update the consumer cursor.
- [Medium] The fallback release function `smc_rx_spd_release()` fails to free the dynamically allocated `smc_spd_priv` object when `splice_to_pipe()` encounters an error or partial push.
--

--- Patch [2]: [PATCH net v5 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg() ---
commit 5ea7024a42bcb5dcdb8d823a5c82e7b6634aa7d2
Author: Bryam Vargas <hexlabsecurity@proton.me>

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

This commit bounds the receive length to the RMB length to prevent
out-of-bounds reads when a forged cursor causes a negative or oversized
accumulator. It clamps the local readable variable to the RMB size.

> 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: High]
This isn't a bug introduced by this patch, but does this clamping leave the
underlying atomic counter corrupted?

By clamping the local readable variable here, the immediate read is bounded,
but the atomic counter conn->bytes_to_rcv remains negative. Later in
smc_rx_recvmsg(), the code does:

    atomic_sub(copylen, &conn->bytes_to_rcv);

Doesn't this subtract the clamped copylen from an already negative value,
driving it further away from the intended invariant?

If the counter remains negative, won't functions like smc_poll() continuously
see a non-zero value and signal EPOLLIN, causing an infinite read loop?
Additionally, won't the SIOCINQ ioctl in smc_ioctl() return this negative
value directly to userspace?

[Severity: Critical]
This is also a pre-existing issue, but does leaving bytes_to_rcv negative
allow a MSG_PEEK operation to trigger an out-of-bounds read?

If a caller uses MSG_PEEK, the atomic_sub() is skipped, and peeked_bytes
can continuously grow by rmb_desc->len. When peeked_bytes is added to the
cursor:

    if ((flags & MSG_PEEK) && peeked_bytes)
        smc_curs_add(conn->rmb_desc->len, &cons, peeked_bytes);

Looking at smc_curs_add():

    curs->count += value;
    if (curs->count >= size) {
        curs->wrap++;
        curs->count -= size;
    }

Because this uses a single if statement rather than a loop or modulo, if the
added value is greater than or equal to 2 * size, doesn't curs->count remain
larger than size (rmb_desc->len)?

If cons.count exceeds rmb_desc->len, the calculation for chunk_len:

    chunk_len = min_t(size_t, copylen, conn->rmb_desc->len - cons.count);

would underflow, resulting in a massive size_t value and bypassing bounds
checks for the subsequent copy.

[ ... ]
>  		chunk_len_sum = chunk_len;
>  		chunk_off = cons.count;
>  		smc_rmb_sync_sg_for_cpu(conn);
>  		for (chunk = 0; chunk < 2; chunk++) {
>  			if (!(flags & MSG_TRUNC)) {
>  				if (msg) {
>  					rc = memcpy_to_msg(msg, rcvbuf_base +
>  							   chunk_off,
>  							   chunk_len);
>  				} else {
>  					rc = smc_rx_splice(pipe, rcvbuf_base +
>  							chunk_off, chunk_len,
>  							smc);
>  				}

[Severity: Critical]
This is a pre-existing issue, but does smc_rx_splice() have a race condition
and refcount mismatch?

Inside smc_rx_splice(), the code calls splice_to_pipe() before acquiring
references:

    bytes = splice_to_pipe(pipe, &spd);
    if (bytes > 0) {
        sock_hold(&smc->sk);
        ...
        get_page(smc->conn.rmb_desc->pages);
    }

Once splice_to_pipe() exposes the pages to the pipe, a concurrent pipe
reader can consume the buffer and trigger the asynchronous release handler,
smc_rx_pipe_buf_release():

    out:
        kfree(priv);
        put_page(buf->page);
        sock_put(sk);

Since the pipe reader can invoke the release handler before the producing
thread acquires the references with sock_hold() and get_page(), can this
cause a use-after-free?

Additionally, smc_rx_pipe_buf_release() drops the socket reference once per
page, but smc_rx_splice() only takes the socket reference once per splice.
Doesn't this guarantee a reference count underflow for multi-page splices?

[Severity: Medium]
This is a pre-existing issue, but does the fallback release function leak
the private data?

When splice_to_pipe() encounters an error or partial push, it invokes the
fallback handler smc_rx_spd_release():

    static void smc_rx_spd_release(struct splice_pipe_desc *spd,
                                   unsigned int i)
    {
        put_page(spd->pages[i]);
    }

This drops the page reference, but it lacks the kfree() for the dynamically
allocated private data (such as kfree((struct smc_spd_priv *)
spd->partial[i].private)). Will this cause a memory leak if the pipe is full?

>  				if (rc < 0) {
>  					if (!read_done)
>  						read_done = -EFAULT;
>  					goto out;
>  				}

[Severity: High]
This is a pre-existing issue, but does this error path cause silent data
corruption?

When reading across a ring buffer wrap, the copy is split into two chunks.
If the first chunk succeeds but the second chunk fails (for example, if a
local application provides a buffer that causes a page fault, returning
-EFAULT), the code takes this goto out path.

Because it jumps to out, it returns read_done (the successfully read first
chunk) but completely skips the cursor update logic at the end of the loop.

Since the socket's internal cursor is not advanced, won't the exact same data
be duplicated to the user on the next read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-disp-0d07164f-v5-0-6a9e235dbc4e@proton.me?part=2

[PATCH net v5 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

[Bryam Vargas]

On the SMC-D DMB-merge (nocopy) path, smc_cdc_msg_recv_action()
advances conn->sndbuf_space from the peer's wire-controlled consumer
cursor via smc_curs_diff(), which can return more than sndbuf_desc->len;
a forged cursor drives sndbuf_space past the send buffer, and over many
CDC messages overflows the signed counter negative. smc_tx_sendmsg()
reads it as the write space and does a wrap-around copy whose second
chunk is not re-bounded to sndbuf_desc->len, spilling the local
sender's outbound data past the send buffer at a peer-controlled
length: a heap out-of-bounds write. The nearby len > sndbuf_desc->len
test only feeds SMC_STAT_RMB_TX_SIZE_SMALL on the user length; it does
not bound the copy.

Bound the write space to sndbuf_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 <= sndbuf_space <= sndbuf_desc->len invariant where it is race-free
against the CDC tasklet; conforming peers are unaffected.

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>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
 net/smc/smc_tx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3144b4b1fe29..5916f02060fb 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -233,6 +233,19 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
 		/* initialize variables for 1st iteration of subsequent loop */
 		/* could be just 1 byte, even after smc_tx_wait above */
 		writespace = atomic_read(&conn->sndbuf_space);
+		/* sndbuf_space is advanced from the peer's wire-controlled
+		 * consumer cursor on the SMC-D DMB-merge path; a forged cursor
+		 * can inflate it past the send buffer, 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 send buffer in
+		 * either case so the wrap-around write cannot run past
+		 * sndbuf_desc->len.  This enforces the documented
+		 * 0 <= sndbuf_space <= sndbuf_desc->len invariant at the
+		 * producer, race-free against the CDC tasklet.
+		 */
+		if (writespace < 0 || writespace > conn->sndbuf_desc->len)
+			writespace = conn->sndbuf_desc->len;
 		/* not more than what user space asked for */
 		copylen = min_t(size_t, send_remaining, writespace);
 		/* determine start of sndbuf */

-- 
2.55.0

[PATCH net v5 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

On the SMC-D DMB-merge (nocopy) path, smc_cdc_msg_recv_action()
advances conn->sndbuf_space from the peer's wire-controlled consumer
cursor via smc_curs_diff(), which can return more than sndbuf_desc->len;
a forged cursor drives sndbuf_space past the send buffer, and over many
CDC messages overflows the signed counter negative. smc_tx_sendmsg()
reads it as the write space and does a wrap-around copy whose second
chunk is not re-bounded to sndbuf_desc->len, spilling the local
sender's outbound data past the send buffer at a peer-controlled
length: a heap out-of-bounds write. The nearby len > sndbuf_desc->len
test only feeds SMC_STAT_RMB_TX_SIZE_SMALL on the user length; it does
not bound the copy.

Bound the write space to sndbuf_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 <= sndbuf_space <= sndbuf_desc->len invariant where it is race-free
against the CDC tasklet; conforming peers are unaffected.

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>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
 net/smc/smc_tx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3144b4b1fe29..5916f02060fb 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -233,6 +233,19 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
 		/* initialize variables for 1st iteration of subsequent loop */
 		/* could be just 1 byte, even after smc_tx_wait above */
 		writespace = atomic_read(&conn->sndbuf_space);
+		/* sndbuf_space is advanced from the peer's wire-controlled
+		 * consumer cursor on the SMC-D DMB-merge path; a forged cursor
+		 * can inflate it past the send buffer, 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 send buffer in
+		 * either case so the wrap-around write cannot run past
+		 * sndbuf_desc->len.  This enforces the documented
+		 * 0 <= sndbuf_space <= sndbuf_desc->len invariant at the
+		 * producer, race-free against the CDC tasklet.
+		 */
+		if (writespace < 0 || writespace > conn->sndbuf_desc->len)
+			writespace = conn->sndbuf_desc->len;
 		/* not more than what user space asked for */
 		copylen = min_t(size_t, send_remaining, writespace);
 		/* determine start of sndbuf */

-- 
2.55.0

Re: [PATCH net v5 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

[sashiko-bot]

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

Pre-existing issues:
- [High] If `memcpy_from_msg` fails during the second chunk of a wrapped copy, `smc_tx_sendmsg` returns a positive `send_done` to user space indicating partial success, but fails to advance the transmit cursors for the successfully copied first chunk, causing silent data loss.
--

--- Patch [3]: [PATCH net v5 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() ---
commit 278a0035d635d6fbcc8028cbd039373266061251
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

This commit bounds the write space to the send buffer length at the consumer
in the SMC-D DMB-merge path. This prevents a forged cursor from inflating
sndbuf_space past the send buffer or overflowing the signed accumulator to a
negative value.

> diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
> index 3144b4b1fe290..5916f02060fb9 100644
> --- a/net/smc/smc_tx.c
> +++ b/net/smc/smc_tx.c
> @@ -233,6 +233,19 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
>  		/* initialize variables for 1st iteration of subsequent loop */
>  		/* could be just 1 byte, even after smc_tx_wait above */
>  		writespace = atomic_read(&conn->sndbuf_space);
> +		/* sndbuf_space is advanced from the peer's wire-controlled
> +		 * consumer cursor on the SMC-D DMB-merge path; a forged cursor
> +		 * can inflate it past the send buffer, 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 send buffer in
> +		 * either case so the wrap-around write cannot run past
> +		 * sndbuf_desc->len.  This enforces the documented
> +		 * 0 <= sndbuf_space <= sndbuf_desc->len invariant at the
> +		 * producer, race-free against the CDC tasklet.
> +		 */
> +		if (writespace < 0 || writespace > conn->sndbuf_desc->len)
> +			writespace = conn->sndbuf_desc->len;
>  		/* not more than what user space asked for */
>  		copylen = min_t(size_t, send_remaining, writespace);
>  		/* determine start of sndbuf */

[Severity: High]
This is a pre-existing issue, but if memcpy_from_msg() fails during the
second chunk of a wrapped copy, can this return a positive send_done to
user space while skipping the cursor updates for the first chunk?

Looking further down in smc_tx_sendmsg(), the copy is done in two chunks:

		for (chunk = 0; chunk < 2; chunk++) {
			rc = memcpy_from_msg(sndbuf_base + chunk_off,
					     msg, chunk_len);
			if (rc) {
				smc_sndbuf_sync_sg_for_device(conn);
				if (send_done)
					return send_done;
				goto out_err;
			}
			send_done += chunk_len;
			...

If the first chunk (chunk == 0) succeeds, send_done is incremented.
If the second chunk (chunk == 1) fails, it hits the if (rc) block and
returns send_done early.

Because it returns early, it skips the cursor and space updates that
normally happen after the loop:

		smc_curs_add(conn->sndbuf_desc->len, &prep, copylen);
		...
		atomic_sub(copylen, &conn->sndbuf_space);

Will this cause the successfully copied first chunk to be overwritten on
the next send operation, leading to silent data loss since user space
believes those bytes were successfully sent?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-disp-0d07164f-v5-0-6a9e235dbc4e@proton.me?part=3