mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers
@ 2026-10-08  6:12 Bryam Vargas via B4 Relay
  2026-10-08  6:12 ` [PATCH net v6 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-10-08  6:12 UTC (permalink / raw)
  To: Sidraya Jayagond, D. Wythe, Eric Dumazet, Mahanta Jambigi,
	Dust Li, David S. Miller, Paolo Abeni, Jakub Kicinski
  Cc: Stefan Raspl, Tony Lu, Hidayath Khan, Ursula Braun, netdev,
	linux-kernel, Simon Horman, linux-s390, linux-rdma, Wen Gu,
	Wenjia Zhang

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: on the
rx/urgent side an OOB read of adjacent kernel memory, returned to the
receiving process; on the tx side an OOB 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. Each clamp checks the
local copy of the counter that the copy length is then taken from, so the
tasklet advancing the counter cannot get between the check and the use.

The clamp is not subsumed by validating each cursor 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. IOW the
overflow lives in the accumulator, not the cursor. Hidayath Khan's "net/smc:
abort the connection when the peer overruns the RMB" rejects that accumulation
on the receive side at the CDC boundary; these clamps bound each consumer and
do not depend on it.

The nearby readable >= rmb_desc->len / len > sndbuf_desc->len tests only feed
stats 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: 0 hits in 5,000,000 reads.
  - every in-bounds / honest-peer arm: clean.
End to end over SMC-D loopback, re-run 2026-10-08 on v7.3-rc6 with this
series applied (two AF_SMC sockets, the sender forging wrap++/count=0,
rmb_desc->len 65504; the bound comes from 2/3 itself, not a modeled clamp):
bytes_to_rcv reaches 6*len in both arms. Unpatched, recv() returns 393024
and the second chunk is a 327520-byte read from ring offset 0 whose last
262016 bytes lie past the RMB; KASAN reports it from smc_rx_recvmsg as
use-after-free in _copy_to_iter (the 2026-07-11 run on 7.2-rc1 read the
same bytes and was labelled slab-use-after-free -- the label follows
whatever sits after the RMB). Patched, recv() returns 65504 and KASAN
stays quiet; an honest transfer is clean.

Changes since v5:
  - repost only: rebased on net (v7.3-rc6), no functional change. Carries Sidraya
    Jayagond's Reviewed-by on all three patches, alongside Dust Li's.
  - 2/3 commit message: the over-read is returned to the receiving process,
    not disclosed to the peer as v5 said. Commit message only.
  - end-to-end A/B re-run on v7.3-rc6 with the v6 patches themselves as the
    fixed arm (above).
  - cover: the receive-side accumulator abort is Hidayath Khan's patch, linked
    below; the v5 cover described it as a separate series of mine.
  v5: https://lore.kernel.org/all/20260723-b4-disp-0d07164f-v5-0-6a9e235dbc4e@proton.me/
  abort: https://lore.kernel.org/all/20260804141109.542202-1-hidayath@linux.ibm.com/

Changes since v4:
  - repost only: the netdev patch queue overflowed, so v5 reposted 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 validate/abort
    change, 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: 6d25ffca055a77787c21a36b66c253f76239411b
change-id: 20261008-b4-disp-f7cd3d96-ff529d85131e

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net v6 1/3] net/smc: bound the wire-controlled producer cursor to the RMB
  2026-10-08  6:12 [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
@ 2026-10-08  6:12 ` Bryam Vargas via B4 Relay
  2026-10-08  6:12 ` [PATCH net v6 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg() Bryam Vargas via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-10-08  6:12 UTC (permalink / raw)
  To: Sidraya Jayagond, D. Wythe, Eric Dumazet, Mahanta Jambigi,
	Dust Li, David S. Miller, Paolo Abeni, Jakub Kicinski
  Cc: Stefan Raspl, Tony Lu, Hidayath Khan, Ursula Braun, netdev,
	linux-kernel, Simon Horman, linux-s390, linux-rdma, Wen Gu,
	Wenjia Zhang

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>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.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.56.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net v6 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg()
  2026-10-08  6:12 [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
  2026-10-08  6:12 ` [PATCH net v6 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
@ 2026-10-08  6:12 ` Bryam Vargas via B4 Relay
  2026-10-08  6:12 ` [PATCH net v6 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() Bryam Vargas via B4 Relay
  2026-10-08  6:15 ` [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers netdev-bot+sinfo
  3 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-10-08  6:12 UTC (permalink / raw)
  To: Sidraya Jayagond, D. Wythe, Eric Dumazet, Mahanta Jambigi,
	Dust Li, David S. Miller, Paolo Abeni, Jakub Kicinski
  Cc: Stefan Raspl, Tony Lu, Hidayath Khan, Ursula Braun, netdev,
	linux-kernel, Simon Horman, linux-s390, linux-rdma, Wen Gu,
	Wenjia Zhang

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
returning it to the receiving process. 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>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.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 197fddc6271f..f781d7229185 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.56.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net v6 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()
  2026-10-08  6:12 [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
  2026-10-08  6:12 ` [PATCH net v6 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
  2026-10-08  6:12 ` [PATCH net v6 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg() Bryam Vargas via B4 Relay
@ 2026-10-08  6:12 ` Bryam Vargas via B4 Relay
  2026-10-08  6:15 ` [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers netdev-bot+sinfo
  3 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-10-08  6:12 UTC (permalink / raw)
  To: Sidraya Jayagond, D. Wythe, Eric Dumazet, Mahanta Jambigi,
	Dust Li, David S. Miller, Paolo Abeni, Jakub Kicinski
  Cc: Stefan Raspl, Tony Lu, Hidayath Khan, Ursula Braun, netdev,
	linux-kernel, Simon Horman, linux-s390, linux-rdma, Wen Gu,
	Wenjia Zhang

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>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.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.56.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers
  2026-10-08  6:12 [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
                   ` (2 preceding siblings ...)
  2026-10-08  6:12 ` [PATCH net v6 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() Bryam Vargas via B4 Relay
@ 2026-10-08  6:15 ` netdev-bot+sinfo
  3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-10-08  6:15 UTC (permalink / raw)
  To: hexlabsecurity
  Cc: Sidraya Jayagond, D. Wythe, Eric Dumazet, Mahanta Jambigi,
	Dust Li, David S. Miller, Paolo Abeni, Jakub Kicinski,
	Stefan Raspl, Tony Lu, Hidayath Khan, Ursula Braun, netdev,
	linux-kernel, Simon Horman, linux-s390, linux-rdma, Wen Gu,
	Wenjia Zhang

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-08  6:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08  6:12 [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
2026-10-08  6:12 ` [PATCH net v6 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
2026-10-08  6:12 ` [PATCH net v6 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg() Bryam Vargas via B4 Relay
2026-10-08  6:12 ` [PATCH net v6 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() Bryam Vargas via B4 Relay
2026-10-08  6:15 ` [PATCH net v6 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers netdev-bot+sinfo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®