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

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

[Bryam Vargas via B4 Relay]

A peer's CDC cursors come off the wire and are used, with no bound
against the local buffers, as a raw index into the RMB on the urgent
path (1/4), as the rx length (2/4), as the tx write space (3/4), and as
the cursor advance at SMC-R link failover (4/4). Any peer that completes
the SMC handshake controls them. With a forged cursor each use runs past
its buffer: an OOB read of kernel memory returned to the reader, send()
data written past the send buffer, or, at failover, an RDMA write whose
source lies past the send buffer. Two of these also happen with a
conforming peer: a MSG_PEEK|MSG_WAITALL reader racing another recv()
(2/4), and late completions on the old link after a failover (3/4).

With a second link in the group, the peer can force the failover: a
DELETE LINK request, or failing one of our RDMA writes, takes the link
down and switches the connections. Late completions need only ACKs on
the old link that arrive after the switch, from a degrading link or a
peer that delays them.

Bounding each cursor on arrival is not enough. A peer that only bumps
prod.wrap with count 0 hits the differing-wrap branch of
smc_curs_diff(), which returns the full buffer length on every CDC, so
bytes_to_rcv grows past the RMB while every cursor looks in range. IOW
the overflow is in the accumulator, not the cursor. 1/4 can clamp on
arrival because the urgent path indexes with the count itself; 2/4-4/4
bound the value where it is used. Hidayath Khan's "net/smc: abort the
connection when the peer overruns the RMB" [1] (net-next) drops the
connection at CDC receive; this series does not depend on it.

Found by inspection of the cursor handling in net/smc; each case was
then confirmed on a KASAN rig with one end patched to forge its
cursors.

Tested on v7.3-rc6 with KASAN, over SMC-D loopback and over SMC-R on
rxe with two links, one end patched to forge its CDC cursors:
  - unpatched: recv(), MSG_PEEK|MSG_WAITALL, SO_RCVLOWAT and splice()
    read past the RMB, as does recv() once bytes_to_rcv wraps
    negative; the urgent byte comes from past the RMB; send() writes
    past the send buffer; after a failover with a forged consumer
    cursor the next RDMA write takes its source past the send buffer
    (rxe refuses that WR).
  - conforming peers, unpatched: the peek race reads past the RMB; a
    completion held back past the failover (a test hook makes the
    timing deterministic) leaves sndbuf_space at 3x the buffer, and
    send() writes past the send buffer.
  - v6 still reads past the RMB on a peek longer than two ring lengths
    and on repeated undrained splices.
  - v7: each case above stays inside its buffer, with no KASAN report,
    and no KMSAN report on the loopback runs.
  - dropping any one patch, or any one part of 2/4, makes a test fail
    again.
  - conforming traffic is unchanged: streams compared byte for byte,
    peek then recv, SO_RCVLOWAT, splice drained and undrained, urgent
    data inline and out-of-band, deferred accept, SIOCINQ and poll,
    nonblocking reads, close with unread data, failover mid-stream, a
    reader waiting while the link group is torn down.

v8 re-run on v7.3 with KASAN over SMC-D loopback (unpatched, v7, v8):
the SMC-D cases above give the same results (v8 as v7), plus the
poll(), SIOCINQ and deferred accept cases under 2/4 below, each with a
mutant that keeps its one site raw and fails. Not re-run over SMC-R: v8
changes only af_smc.c's code, the rest are comment lines; smc_rx.o,
smc_tx.o and smc_core.o built from v8 are byte-identical to v7's on the
same tree. Logs on request.

Changes in v8:
  - 2/4: smc_poll(), SIOCINQ and the deferred accept read bytes_to_rcv
    through the bounded helper too (Sashiko's review of v7 [8]). With
    the count driven negative (wrap bumps past INT_MAX), v7's poll()
    said POLLIN while recv() gave -EAGAIN, so a level-triggered reader
    spun (1,887,464 wakeups and 1000 ms of CPU in 1 s; v8: 0 wakeups),
    SIOCINQ returned the negative count (v8: 0), and a deferred accept
    returned at once with nothing to read (v8 waits out its timeout).
    With the count at 6x the RMB, SIOCINQ said 6x (v8: the RMB).
    Conforming traffic sees no change. The two comments that claimed
    0 <= bytes_to_rcv <= len say "can be out of range" now.
  - 3/4: code unchanged; the comment that claimed 0 <= sndbuf_space <=
    len points at the clamp instead [9]. R-b kept.
  - 4/4: code unchanged; the catch-up comment now says why it is bounded
    (a peer past the bound stalls). The message no longer claims the
    destination side: smc_write_space() still sizes the window from the
    peer's cursor, so a forged one can take the next write past the
    peer's own RMB, where its HCA refuses it [10].
  - Rebased on net after the net-next merge (fcc00b15cb21).
  v7: https://lore.kernel.org/all/20261013-b4-disp-f345196c-v7-0-ef3dddfbfa68@proton.me/

Changes in v7:
  - 2/4 (was v6 2/3): per Sashiko's reviews of v6 [2][3][5], MSG_PEEK
    and splice() offsets could start the copy outside the RMB, a local
    peek race [3] could leave the available length negative, and wrap
    bumps grow bytes_to_rcv (the accumulator case above). The bound
    moves into smc_rx_data_available(), counts peeked and spliced
    bytes, maps a negative length to 0, not a full ring, and covers
    the wait predicates too. The RMB length is cached in conn->rmb_len
    for the unlocked wait, and splice_pending now drops before the
    consumer cursor moves. New Fixes:, Reported-by: and Closes: tags;
    R-b dropped.
  - 4/4 is new: the failover cursor advance, which 3/4 does not cover.
  - 3/4: code unchanged. [4] noted that DMB merge runs only on the
    loopback device, so its peer is the local kernel; the message now
    leads with the over-credit after a failover, which needs no forged
    cursor, and says it bounds the copy. New Fixes:, R-b kept.
  - 1/4: code unchanged, comment and message reworded. R-b kept.
  - [6] (rmb_desc NULL before smc_buf_create()) is pre-existing; my
    fix for it, [7], is marked changes requested in patchwork and
    touches the same lines of __smc_buf_create() as 2/4.
  v6: https://lore.kernel.org/all/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c@proton.me/

Changes in v6: repost on net; 2/3 message corrected (the over-read goes
to the reader, not the peer); Sidraya Jayagond's R-b.
  v5: https://lore.kernel.org/all/20260723-b4-disp-0d07164f-v5-0-6a9e235dbc4e@proton.me/

Changes in v5: repost of v4.
  v4: https://lore.kernel.org/all/20260705-b4-disp-28a1bbca-v4-0-be089b98acc6@proton.me/

Changes in v4: split off the validate/abort change, per Dust Li.
  v3: https://lore.kernel.org/all/20260614-b4-disp-edd64be9-v3-0-551fa514257e@proton.me/

[1] https://lore.kernel.org/all/20260804141109.542202-1-hidayath@linux.ibm.com/
[2] https://lore.kernel.org/all/sashiko-outbox-164918@kernel.org/
[3] https://lore.kernel.org/all/179152807838.434549.2207466107170796457@kernel.org/
[4] https://lore.kernel.org/all/179152807992.434549.3295281690534839972@kernel.org/
[5] https://lore.kernel.org/all/179152807674.434549.14168488606722848951@kernel.org/
[6] https://lore.kernel.org/all/sashiko-outbox-164917@kernel.org/
[7] https://lore.kernel.org/all/20260728-b4-disp-52ee4e7d-v4-1-0dda94b0f397@proton.me/
[8] https://lore.kernel.org/all/179209532096.434549.4189685148052558990@kernel.org/
[9] https://lore.kernel.org/all/179209532279.434549.3831566859438447501@kernel.org/
[10] https://lore.kernel.org/all/179209532452.434549.6588379141939967966@kernel.org/

---
Bryam Vargas (4):
      net/smc: bound the wire-controlled producer cursor to the RMB
      net/smc: bound the receive length to the RMB
      net/smc: bound the send length to the send buffer in smc_tx_sendmsg()
      net/smc: bound the link failover cursor advance to the send buffer

 net/smc/af_smc.c   |  6 +++---
 net/smc/smc.h      |  3 +++
 net/smc/smc_cdc.c  |  2 +-
 net/smc/smc_cdc.h  | 17 ++++++++++++++---
 net/smc/smc_core.c | 19 +++++++++++++------
 net/smc/smc_rx.c   |  9 +++++++--
 net/smc/smc_rx.h   | 11 ++++++++++-
 net/smc/smc_tx.c   | 10 +++++++++-
 8 files changed, 60 insertions(+), 17 deletions(-)
---
base-commit: fcc00b15cb21a6de76826e5012b2c758505f5e65
change-id: 20261029-b4-disp-33001ace-db9b3e0669a6

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

[PATCH net v8 1/4] 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 the peer's
producer cursor from the wire into conn->local_rx_ctrl.prod without
bounding it to the RMB. smc_cdc_handle_urg_data_arrival() then uses its
count as a raw index into the RMB, so a producer cursor past
rmb_desc->len makes the receive tasklet read past the RMB, and
recv(MSG_OOB) hands that byte to the application.

Bound the producer cursor count to rmb_desc->len when it comes off the
wire, for SMC-R and SMC-D. Leave the consumer cursor alone: it indexes
the peer's RMB, which can be larger than ours, so clamping it to our
RMB would under-credit peer_rmbe_space and could stall sending to that
peer. 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>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>
---
 net/smc/smc_cdc.h | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 696cc11f2303..4d630a305118 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,12 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
 	if ((old.wrap == temp.wrap) &&
 	    (old.count > temp.count))
 		return;
+	/* The urgent path uses the producer count as a raw index into our
+	 * RMB, so bound it. max_count == 0 leaves the consumer cursor alone:
+	 * it indexes the peer's RMB, which can be larger than ours.
+	 */
+	if (max_count && temp.count > max_count)
+		temp.count = max_count;
 	smc_curs_copy(local, &temp, conn);
 }
 
@@ -246,8 +253,9 @@ 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);
+	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 +268,9 @@ static inline void smcd_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 
 	temp.wrap = peer->prod.wrap;
 	temp.count = peer->prod.count;
+	/* as in smc_cdc_cursor_to_host(): bound prod, not cons */
+	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.56.0

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

[sashiko-bot]

> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> smcr_cdc_msg_to_host() and smcd_cdc_msg_to_host() import the peer's
> producer cursor from the wire into conn->local_rx_ctrl.prod without
> bounding it to the RMB. smc_cdc_handle_urg_data_arrival() then uses its
> count as a raw index into the RMB, so a producer cursor past
> rmb_desc->len makes the receive tasklet read past the RMB, and
> recv(MSG_OOB) hands that byte to the application.
> 
> Bound the producer cursor count to rmb_desc->len when it comes off the
> wire, for SMC-R and SMC-D. Leave the consumer cursor alone: it indexes
> the peer's RMB, which can be larger than ours, so clamping it to our
> RMB would under-credit peer_rmbe_space and could stall sending to that
> peer. Conforming peers are unaffected.
> 
> Fixes: de8474eb9d50 ("net/smc: urgent data support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-b4-disp-33001ace-v8-0-f9bb96e17827@proton.me?part=1

[PATCH net v8 2/4] net/smc: bound the receive length to the RMB

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

smc_rx_recvmsg() sizes the copy from bytes_to_rcv and starts it past the
bytes MSG_PEEK or splice() already took. A forged producer cursor drives
bytes_to_rcv past the RMB or negative, and with a conforming peer it
drops below the peeked count when a MSG_PEEK|MSG_WAITALL reader sleeps
while another recv() consumes what it peeked. Either way the copy reads
past the RMB into the reader's buffer: the second chunk runs long, or
the start offset leaves the RMB, as smc_curs_add() wraps only once.
poll(), SIOCINQ and the deferred accept read the same raw count, so
SIOCINQ hands it to userspace as is, and bounding recv() alone would
leave a level-triggered reader spinning on a socket recv() finds empty.

Make smc_rx_data_available() report at most what fits in the RMB behind
the peeked and spliced bytes, never a negative count, and use it for the
copy, the wait predicates, poll(), SIOCINQ and the deferred accept, so
they all see the same length. Cache the RMB length for the unlocked
wait, and drop released bytes from splice_pending before the consumer
cursor moves past them, so no reader counts them twice.

Fixes: 952310ccf2d8 ("smc: receive data from RMBE")
Fixes: 9014db202cb7 ("smc: add support for splice()")
Fixes: a4b6539038c1 ("net/smc: fix data error when recvmsg with MSG_PEEK flag")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c@proton.me?part=2
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c%40proton.me?part=2
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c%40proton.me?part=1
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
The wait predicates run unlocked, where a link group termination may
already have freed rmb_desc, hence the cached length. With the old
splice_pending order, a reader that took the lock between the cursor
update and the drop counted the bytes twice, and the next splice()
returned stale bytes in place of unread data. 1/4 bounds the producer
count, but wrap bumps still grow bytes_to_rcv past the RMB (see the
cover).
---
 net/smc/af_smc.c   |  6 +++---
 net/smc/smc.h      |  3 +++
 net/smc/smc_cdc.c  |  2 +-
 net/smc/smc_core.c |  1 +
 net/smc/smc_rx.c   |  9 +++++++--
 net/smc/smc_rx.h   | 11 ++++++++++-
 6 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index d10244af5bd2..d492c7716128 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -2774,7 +2774,7 @@ int smc_accept(struct socket *sock, struct socket *new_sock,
 			if (skb_queue_empty(&clcsk->sk_receive_queue))
 				sk_wait_data(clcsk, &timeo, NULL);
 			release_sock(clcsk);
-		} else if (!atomic_read(&smc_sk(nsk)->conn.bytes_to_rcv)) {
+		} else if (!smc_rx_data_available(&smc_sk(nsk)->conn, 0)) {
 			lock_sock(nsk);
 			smc_rx_wait(smc_sk(nsk), &timeo, 0, smc_rx_data_available);
 			release_sock(nsk);
@@ -2939,7 +2939,7 @@ __poll_t smc_poll(struct file *file, struct socket *sock,
 						mask |= EPOLLOUT | EPOLLWRNORM;
 				}
 			}
-			if (atomic_read(&smc->conn.bytes_to_rcv))
+			if (smc_rx_data_available(&smc->conn, 0))
 				mask |= EPOLLIN | EPOLLRDNORM;
 			if (sk->sk_shutdown & RCV_SHUTDOWN)
 				mask |= EPOLLIN | EPOLLRDNORM | EPOLLRDHUP;
@@ -3237,7 +3237,7 @@ int smc_ioctl(struct socket *sock, unsigned int cmd,
 		    smc->sk.sk_state == SMC_CLOSED)
 			answ = 0;
 		else
-			answ = atomic_read(&smc->conn.bytes_to_rcv);
+			answ = smc_rx_data_available(&smc->conn, 0);
 		break;
 	case SIOCOUTQ:
 		/* output queue size (not send + not acked) */
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 427b6d63b993..d4325a0a8fd8 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -203,6 +203,9 @@ struct smc_connection {
 						 * rmbe
 						 */
 	int			rtoken_idx;	/* idx to peer RMB rkey/addr */
+	int			rmb_len;	/* rmb_desc->len, still valid
+						 * after rmb_desc is freed
+						 */
 
 	struct smc_buf_desc	*sndbuf_desc;	/* send buffer descriptor */
 	struct smc_buf_desc	*rmb_desc;	/* RMBE descriptor */
diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index 32d6d03df321..e2c765ff2ba3 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -381,7 +381,7 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
 		/* bytes_to_rcv is decreased in smc_recvmsg */
 		smp_mb__before_atomic();
 		atomic_add(diff_prod, &conn->bytes_to_rcv);
-		/* guarantee 0 <= bytes_to_rcv <= rmb_desc->len */
+		/* can be out of range, see smc_rx_data_available() */
 		smp_mb__after_atomic();
 		smc->sk.sk_data_ready(&smc->sk);
 	} else {
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 6021322253e9..fe6e4a2f7935 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -2505,6 +2505,7 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_smcd, bool is_rmb)
 
 	if (is_rmb) {
 		conn->rmb_desc = buf_desc;
+		conn->rmb_len = buf_desc->len;
 		conn->rmbe_size_comp = bufsize_comp;
 		smc->sk.sk_rcvbuf = bufsize * 2;
 		atomic_set(&conn->bytes_to_rcv, 0);
diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index 197fddc6271f..85654d9f321b 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -118,15 +118,20 @@ static void smc_rx_pipe_buf_release(struct pipe_inode_info *pipe,
 	struct smc_connection *conn = &priv->smc->conn;
 	struct smc_sock *smc = priv->smc;
 	struct sock *sk = &smc->sk;
+	bool drained;
 
 	lock_sock(sk);
 	if (conn->freed) {
 		release_sock(sk);
 		goto out;
 	}
+	/* drop the released bytes from splice_pending before cons moves
+	 * past them, so no reader counts them twice
+	 */
+	drained = atomic_sub_and_test(priv->len, &conn->splice_pending);
 	smc_rx_update_cons(smc, priv->len);
 	release_sock(sk);
-	if (atomic_sub_and_test(priv->len, &conn->splice_pending))
+	if (drained)
 		smc_rx_wake_up(sk);
 out:
 	kfree(priv);
@@ -506,7 +511,7 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg,
 			/* increased in recv tasklet smc_cdc_msg_rcv() */
 			smp_mb__before_atomic();
 			atomic_sub(copylen, &conn->bytes_to_rcv);
-			/* guarantee 0 <= bytes_to_rcv <= rmb_desc->len */
+			/* can be out of range, see smc_rx_data_available() */
 			smp_mb__after_atomic();
 			if (msg && smc_rx_update_consumer(smc, cons, copylen))
 				goto out;
diff --git a/net/smc/smc_rx.h b/net/smc/smc_rx.h
index 994f5e42d1ba..6bf44e3cc33a 100644
--- a/net/smc/smc_rx.h
+++ b/net/smc/smc_rx.h
@@ -25,7 +25,16 @@ int smc_rx_wait(struct smc_sock *smc, long *timeo, size_t peeked,
 		int (*fcrit)(struct smc_connection *conn, size_t baseline));
 static inline int smc_rx_data_available(struct smc_connection *conn, size_t peeked)
 {
-	return atomic_read(&conn->bytes_to_rcv) - peeked;
+	int avail = atomic_read(&conn->bytes_to_rcv);
+
+	/* bytes_to_rcv follows the peer's producer cursor and can be past
+	 * the RMB or negative. Report no more than fits in the RMB behind the
+	 * bytes already spliced or peeked. Compare before subtracting: near
+	 * INT_MIN, avail - peeked wraps. This also runs unlocked, possibly
+	 * after rmb_desc was freed, hence rmb_len.
+	 */
+	avail = min(avail, conn->rmb_len - atomic_read(&conn->splice_pending));
+	return avail > (int)peeked ? avail - (int)peeked : 0;
 }
 
 #endif /* SMC_RX_H */

-- 
2.56.0

Re: [PATCH net v8 2/4] net/smc: bound the receive length to the RMB

[sashiko-bot]

> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> smc_rx_recvmsg() sizes the copy from bytes_to_rcv and starts it past the
> bytes MSG_PEEK or splice() already took. A forged producer cursor drives
> bytes_to_rcv past the RMB or negative, and with a conforming peer it
> drops below the peeked count when a MSG_PEEK|MSG_WAITALL reader sleeps
> while another recv() consumes what it peeked. Either way the copy reads
> past the RMB into the reader's buffer: the second chunk runs long, or
> the start offset leaves the RMB, as smc_curs_add() wraps only once.
> poll(), SIOCINQ and the deferred accept read the same raw count, so
> SIOCINQ hands it to userspace as is, and bounding recv() alone would
> leave a level-triggered reader spinning on a socket recv() finds empty.
> 
> Make smc_rx_data_available() report at most what fits in the RMB behind
> the peeked and spliced bytes, never a negative count, and use it for the
> copy, the wait predicates, poll(), SIOCINQ and the deferred accept, so
> they all see the same length. Cache the RMB length for the unlocked
> wait, and drop released bytes from splice_pending before the consumer
> cursor moves past them, so no reader counts them twice.
> 
> Fixes: 952310ccf2d8 ("smc: receive data from RMBE")
> Fixes: 9014db202cb7 ("smc: add support for splice()")
> Fixes: a4b6539038c1 ("net/smc: fix data error when recvmsg with MSG_PEEK flag")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c@proton.me?part=2
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c%40proton.me?part=2
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c%40proton.me?part=1
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-b4-disp-33001ace-v8-0-f9bb96e17827@proton.me?part=2

[PATCH net v8 3/4] 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>

conn->sndbuf_space can run past the send buffer. After an SMC-R link
failover, a late completion from the old link can move tx_curs_fin back,
and the next completion credits the same bytes again; across the ring
end, smc_curs_diff() credits the step back itself as more than a whole
ring, so even with a conforming peer sndbuf_space can reach three times
the buffer. The failover catch-up and SMC-D DMB merge (only the loopback
device today) also credit it from the peer's consumer cursor without a
bound. smc_tx_sendmsg() takes it as the write space, and the second
chunk of its copy writes past the send buffer.

Clamp writespace, which sizes the copy, to sndbuf_desc->len, negative
values included, so the copy stays in the send buffer.

Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
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>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>
---
This keeps the copy inside the send buffer but does not undo the
over-credit, so after such a failover send() can still overwrite data in
the buffer that the peer has not received yet (by inspection; not
tested). Fixing the credit in smc_cdc_tx_handler() needs a cursor
compare that survives the u16 wrap rollover, which smc_curs_comp() does
not do, so it is left out here. A negative value maps to len, not 0,
since 0 would make the send loop spin.
---
 net/smc/smc_tx.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 0f0c6df3bf2c..3913ef103b35 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -233,6 +233,14 @@ 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 can be out of range: late completions of an
+		 * old SMC-R link credit it again after a failover, and with
+		 * SMC-D DMB merge it follows the peer's consumer cursor.
+		 * Bound writespace, which sizes the copy; a negative value
+		 * would become huge in the size_t min_t() below.
+		 */
+		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 */
@@ -278,7 +286,7 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
 		/* increased in send tasklet smc_cdc_tx_handler() */
 		smp_mb__before_atomic();
 		atomic_sub(copylen, &conn->sndbuf_space);
-		/* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
+		/* can be out of range, see writespace above */
 		smp_mb__after_atomic();
 		/* since we just produced more new data into sndbuf,
 		 * trigger sndbuf consumer: RDMA write into peer RMBE and CDC

-- 
2.56.0

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

[sashiko-bot]

> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> conn->sndbuf_space can run past the send buffer. After an SMC-R link
> failover, a late completion from the old link can move tx_curs_fin back,
> and the next completion credits the same bytes again; across the ring
> end, smc_curs_diff() credits the step back itself as more than a whole
> ring, so even with a conforming peer sndbuf_space can reach three times
> the buffer. The failover catch-up and SMC-D DMB merge (only the loopback
> device today) also credit it from the peer's consumer cursor without a
> bound. smc_tx_sendmsg() takes it as the write space, and the second
> chunk of its copy writes past the send buffer.
> 
> Clamp writespace, which sizes the copy, to sndbuf_desc->len, negative
> values included, so the copy stays in the send buffer.
> 
> Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-b4-disp-33001ace-v8-0-f9bb96e17827@proton.me?part=3

[PATCH net v8 4/4] net/smc: bound the link failover cursor advance to the send buffer

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

On SMC-R link failover, smc_switch_cursor() rewinds the send and
producer cursors to their values at the last completed CDC send, then
advances them by the distance the peer's consumer cursor is ahead. That
cursor comes from the wire, so a forged one adds an arbitrary amount to
sndbuf_space and to the cursors. smc_curs_add() wraps them only once, so
the next RDMA write takes its source past the send buffer.

A conforming peer keeps its cursor count inside its RMB and cannot
consume past the data in the send buffer. Advance only for such a count,
and no further than tx_curs_prep. The pre-failover tx_curs_sent cannot
be the bound: an earlier failover may have rewound it below the peer's
cursor. The destination side is not bounded here: smc_write_space()
still sizes the window from the peer's cursor, so a forged one can take
the next write past the peer's own RMB, where its HCA refuses it and
the link goes down.

Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Bounding the copy in smc_tx_sendmsg() (3/4) does not cover this: the
RDMA write takes its offsets from the cursors. Measured on rxe, send
buffer 16K, peer RMB 32K, 4096 bytes sent and unconfirmed, the consumer
cursor forged one wrap ahead at count 64: unpatched, tx_curs_sent lands
64 bytes past the send buffer and the next write takes its source from
there (rxe refuses that WR); with the patch the advance stops at
tx_curs_prep, nothing is pending, and nothing is written after the
switch. The destination side, with the consumer cursor forged two wraps
ahead at the peer's RMB + 4096 (send buffer 64K, peer RMB 16K): with the
patch the catch-up is skipped, the window is the whole RMB and the first
write after the switch, from prod.count 1, ends one byte past the peer's
RMB; its HCA refuses the write and the link goes down. Without the patch
the catch-up lands prod on the forged count, smc_write_space() is 0 and
nothing more is written.
---
 net/smc/smc_core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index fe6e4a2f7935..42d83a002ffb 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1034,7 +1034,7 @@ static int smc_switch_cursor(struct smc_sock *smc, struct smc_cdc_tx_pend *pend,
 			     struct smc_wr_buf *wr_buf)
 {
 	struct smc_connection *conn = &smc->conn;
-	union smc_host_cursor cons, fin;
+	union smc_host_cursor cons, fin, prep;
 	int rc = 0;
 	int diff;
 
@@ -1043,12 +1043,18 @@ static int smc_switch_cursor(struct smc_sock *smc, struct smc_cdc_tx_pend *pend,
 	/* set prod cursor to old state, enforce tx_rdma_writes() */
 	smc_curs_copy(&conn->local_tx_ctrl.prod, &fin, conn);
 	smc_curs_copy(&cons, &conn->local_rx_ctrl.cons, conn);
-
-	if (smc_curs_comp(conn->peer_rmbe_size, &cons, &fin) < 0) {
-		/* cons cursor advanced more than fin, and prod was set
-		 * fin above, so now prod is smaller than cons. Fix that.
+	smc_curs_copy(&prep, &conn->tx_curs_prep, conn);
+
+	if (cons.count < conn->peer_rmbe_size &&
+	    smc_curs_comp(conn->peer_rmbe_size, &cons, &fin) < 0) {
+		/* cons cursor advanced more than fin, and prod was set fin
+		 * above, so now prod is smaller than cons. cons comes off the
+		 * wire: catch up only for a count inside the peer's RMB and no
+		 * further than the send buffer holds, else that peer stalls.
 		 */
-		diff = smc_curs_diff(conn->peer_rmbe_size, &fin, &cons);
+		diff = min(smc_curs_diff(conn->peer_rmbe_size, &fin, &cons),
+			   smc_curs_diff(conn->sndbuf_desc->len,
+					 &conn->tx_curs_sent, &prep));
 		smc_curs_add(conn->sndbuf_desc->len,
 			     &conn->tx_curs_sent, diff);
 		smc_curs_add(conn->sndbuf_desc->len,

-- 
2.56.0

Re: [PATCH net v8 4/4] net/smc: bound the link failover cursor advance to the send buffer

[sashiko-bot]

> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> On SMC-R link failover, smc_switch_cursor() rewinds the send and
> producer cursors to their values at the last completed CDC send, then
> advances them by the distance the peer's consumer cursor is ahead. That
> cursor comes from the wire, so a forged one adds an arbitrary amount to
> sndbuf_space and to the cursors. smc_curs_add() wraps them only once, so
> the next RDMA write takes its source past the send buffer.
> 
> A conforming peer keeps its cursor count inside its RMB and cannot
> consume past the data in the send buffer. Advance only for such a count,
> and no further than tx_curs_prep. The pre-failover tx_curs_sent cannot
> be the bound: an earlier failover may have rewound it below the peer's
> cursor. The destination side is not bounded here: smc_write_space()
> still sizes the window from the peer's cursor, so a forged one can take
> the next write past the peer's own RMB, where its HCA refuses it and
> the link goes down.
> 
> Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-b4-disp-33001ace-v8-0-f9bb96e17827@proton.me?part=4