* [PATCH v3 1/3] net/smc: bound the wire-controlled producer cursor to the RMB
2026-06-14 8:23 [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
@ 2026-06-14 8:23 ` Bryam Vargas via B4 Relay
2026-06-18 14:29 ` Dust Li
2026-06-14 8:23 ` [PATCH v3 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, 1 reply; 8+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-14 8:23 UTC (permalink / raw)
To: Wenjia Zhang, Dust Li, D. Wythe, Sidraya Jayagond
Cc: Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Jakub Kicinski, Tony Lu
From: Bryam Vargas <hexlabsecurity@proton.me>
smc_cdc_cursor_to_host() (SMC-R) and smcd_cdc_msg_to_host() (SMC-D)
import the peer's producer cursor from the wire into the local
connection cursor with no upper bound against the receive buffer (RMB).
The urgent path then uses that count as a raw index:
base = conn->rmb_desc->cpu_addr + conn->rx_off;
conn->urg_rx_byte = *(base + conn->urg_curs.count - 1);
so a peer that advertises a producer cursor past rmb_desc->len reads
out of bounds of the RMB allocation in the receive tasklet (softirq).
Bound the producer cursor count to rmb_desc->len at the conversion
boundary, for both SMC-R and SMC-D. Apply the bound to the producer
cursor only: 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 whose RMB is
larger than ours.
Fixes: de8474eb9d50 ("net/smc: urgent data support")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
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.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 1/3] net/smc: bound the wire-controlled producer cursor to the RMB
2026-06-14 8:23 ` [PATCH v3 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
@ 2026-06-18 14:29 ` Dust Li
0 siblings, 0 replies; 8+ messages in thread
From: Dust Li @ 2026-06-18 14:29 UTC (permalink / raw)
To: hexlabsecurity, Wenjia Zhang, D. Wythe, Sidraya Jayagond
Cc: Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Jakub Kicinski, Tony Lu
On 2026-06-14 03:23:30, Bryam Vargas via B4 Relay wrote:
>From: Bryam Vargas <hexlabsecurity@proton.me>
>
>smc_cdc_cursor_to_host() (SMC-R) and smcd_cdc_msg_to_host() (SMC-D)
>import the peer's producer cursor from the wire into the local
>connection cursor with no upper bound against the receive buffer (RMB).
>The urgent path then uses that count as a raw index:
>
> base = conn->rmb_desc->cpu_addr + conn->rx_off;
> conn->urg_rx_byte = *(base + conn->urg_curs.count - 1);
>
>so a peer that advertises a producer cursor past rmb_desc->len reads
>out of bounds of the RMB allocation in the receive tasklet (softirq).
>
>Bound the producer cursor count to rmb_desc->len at the conversion
>boundary, for both SMC-R and SMC-D. Apply the bound to the producer
>cursor only: 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 whose RMB is
>larger than ours.
>
>Fixes: de8474eb9d50 ("net/smc: urgent data support")
>Cc: stable@vger.kernel.org
>Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
>---
> 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;
Hi Bryam,
I agree the issue is real. SMC-R's original design didn't fully
account for misbehaving peers, which is the root cause behind a
number of similar issues we've seen. The good news is that this
class of problem isn't easy to hit in practice, so it isn't
particularly urgent.
On the approach itself: once we detect that the peer is misbehaving,
I think the right action is to abort the connection and record the
event, rather than silently clamp. An invalid CDC means the whole
communication state can no longer be trusted, so continuing on a
clamped value just papers over a peer bug.
I'd suggest we add a dedicated CDC message check, and route any
failure through the existing abort path, maybe something like bellow:
static bool smc_cdc_msg_check(struct smc_connection *conn,
struct smc_cdc_msg *cdc)
{
u32 prod_count = ntohs(cdc->prod.count);
u32 cons_count = ntohs(cdc->cons.count);
if (prod_count > conn->rmb_desc->len ||
cons_count > conn->peer_rmbe_size ||
cdc->prod.wrap > 1 || cdc->cons.wrap > 1) {
this_cpu_inc(net->smc.smc_stats->...cdc_inval);
net_ratelimited_function(pr_warn,
"smc: invalid CDC from peer (token=%u)\n",
ntohl(cdc->token));
return false;
}
return true;
}
For -stable, your current minimal patch is fine. For net-next, though, I'd prefer
the approach above: validate at the wire boundary, abort on violation, and
make the event observable via smc_stats and a ratelimited warning.
Best regards,
Dust
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg()
2026-06-14 8:23 [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
2026-06-14 8:23 ` [PATCH v3 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
@ 2026-06-14 8:23 ` Bryam Vargas via B4 Relay
2026-06-18 16:03 ` Dust Li
2026-06-14 8:23 ` [PATCH v3 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() Bryam Vargas via B4 Relay
2026-06-17 23:24 ` [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Jakub Kicinski
3 siblings, 1 reply; 8+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-14 8:23 UTC (permalink / raw)
To: Wenjia Zhang, Dust Li, D. Wythe, Sidraya Jayagond
Cc: Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Jakub Kicinski, Tony Lu
From: Bryam Vargas <hexlabsecurity@proton.me>
conn->bytes_to_rcv is accumulated in the receive tasklet from the peer's
producer cursor:
diff_prod = smc_curs_diff(rmb_desc->len, &prod_old, &prod_new);
atomic_add(diff_prod, &conn->bytes_to_rcv);
smc_curs_diff()'s differing-wrap branch returns (size - old.count) +
new.count, which exceeds rmb_desc->len for a forged producer cursor and
accumulates across CDC messages, so bytes_to_rcv can grow past the RMB
(and across many messages can overflow the signed counter negative).
smc_rx_recvmsg() reads it as the number of readable bytes and performs a
wrap-around copy whose second chunk (copylen - first_chunk, read from
ring offset 0) is never re-bounded to rmb_desc->len, reading past the
RMB into adjacent kernel memory and disclosing it to the peer.
Bound the readable count to rmb_desc->len where it is consumed, treating
a negative (sign-overflowed) value as out of range too, so the copy
length can never exceed the ring. This enforces the documented
0 <= bytes_to_rcv <= rmb_desc->len invariant at the consumer, where it
is race-free against the producer update that runs in the receive
tasklet.
Fixes: 952310ccf2d8 ("smc: receive data from RMBE")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
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.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg()
2026-06-14 8:23 ` [PATCH v3 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg() Bryam Vargas via B4 Relay
@ 2026-06-18 16:03 ` Dust Li
0 siblings, 0 replies; 8+ messages in thread
From: Dust Li @ 2026-06-18 16:03 UTC (permalink / raw)
To: hexlabsecurity, Wenjia Zhang, D. Wythe, Sidraya Jayagond
Cc: Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Jakub Kicinski, Tony Lu
On 2026-06-14 03:23:31, Bryam Vargas via B4 Relay wrote:
>From: Bryam Vargas <hexlabsecurity@proton.me>
>
>conn->bytes_to_rcv is accumulated in the receive tasklet from the peer's
>producer cursor:
>
> diff_prod = smc_curs_diff(rmb_desc->len, &prod_old, &prod_new);
> atomic_add(diff_prod, &conn->bytes_to_rcv);
>
>smc_curs_diff()'s differing-wrap branch returns (size - old.count) +
>new.count, which exceeds rmb_desc->len for a forged producer cursor and
>accumulates across CDC messages, so bytes_to_rcv can grow past the RMB
>(and across many messages can overflow the signed counter negative).
>smc_rx_recvmsg() reads it as the number of readable bytes and performs a
>wrap-around copy whose second chunk (copylen - first_chunk, read from
>ring offset 0) is never re-bounded to rmb_desc->len, reading past the
>RMB into adjacent kernel memory and disclosing it to the peer.
Hi Bryam,
Once we validate the CDC message at the input boundary (as in the
previous patch), bytes_to_rcv can never exceed rmb_desc->len, so
this check becomes unreachable. So I don't think this patch is needed.
Best regards,
Dust
>
>Bound the readable count to rmb_desc->len where it is consumed, treating
>a negative (sign-overflowed) value as out of range too, so the copy
>length can never exceed the ring. This enforces the documented
>0 <= bytes_to_rcv <= rmb_desc->len invariant at the consumer, where it
>is race-free against the producer update that runs in the receive
>tasklet.
>
>Fixes: 952310ccf2d8 ("smc: receive data from RMBE")
>Cc: stable@vger.kernel.org
>Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
>---
> 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.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()
2026-06-14 8:23 [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
2026-06-14 8:23 ` [PATCH v3 1/3] net/smc: bound the wire-controlled producer cursor to the RMB Bryam Vargas via B4 Relay
2026-06-14 8:23 ` [PATCH v3 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg() Bryam Vargas via B4 Relay
@ 2026-06-14 8:23 ` Bryam Vargas via B4 Relay
2026-06-18 16:08 ` Dust Li
2026-06-17 23:24 ` [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Jakub Kicinski
3 siblings, 1 reply; 8+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-14 8:23 UTC (permalink / raw)
To: Wenjia Zhang, Dust Li, D. Wythe, Sidraya Jayagond
Cc: Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Jakub Kicinski, Tony Lu
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 consumer cursor:
diff_tx = smc_curs_diff(sndbuf_desc->len, &tx_curs_fin,
&local_rx_ctrl.cons);
atomic_add(diff_tx, &conn->sndbuf_space);
The consumer cursor is wire-controlled and unvalidated, and
smc_curs_diff()'s differing-wrap branch can return more than
sndbuf_desc->len, so a forged cursor drives sndbuf_space past the send
buffer (and across many CDC messages can overflow the signed counter
negative). smc_tx_sendmsg() reads it as the available write space and
performs a wrap-around copy whose second chunk (copylen - first_chunk,
written at ring offset 0) is never re-bounded to sndbuf_desc->len,
writing user data past the send buffer -- a heap out-of-bounds write of
attacker-influenced length and content.
Bound the write space to sndbuf_desc->len where it is consumed, treating
a negative (sign-overflowed) value as out of range too, so the copy
length can never exceed the ring. This enforces the documented
0 <= sndbuf_space <= sndbuf_desc->len invariant at the producer,
race-free against the CDC tasklet that advances sndbuf_space.
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>
---
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.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()
2026-06-14 8:23 ` [PATCH v3 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() Bryam Vargas via B4 Relay
@ 2026-06-18 16:08 ` Dust Li
0 siblings, 0 replies; 8+ messages in thread
From: Dust Li @ 2026-06-18 16:08 UTC (permalink / raw)
To: hexlabsecurity, Wenjia Zhang, D. Wythe, Sidraya Jayagond
Cc: Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Jakub Kicinski, Tony Lu
On 2026-06-14 03:23:32, Bryam Vargas via B4 Relay wrote:
>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 consumer cursor:
>
> diff_tx = smc_curs_diff(sndbuf_desc->len, &tx_curs_fin,
> &local_rx_ctrl.cons);
> atomic_add(diff_tx, &conn->sndbuf_space);
>
>The consumer cursor is wire-controlled and unvalidated, and
>smc_curs_diff()'s differing-wrap branch can return more than
>sndbuf_desc->len, so a forged cursor drives sndbuf_space past the send
>buffer (and across many CDC messages can overflow the signed counter
>negative). smc_tx_sendmsg() reads it as the available write space and
>performs a wrap-around copy whose second chunk (copylen - first_chunk,
>written at ring offset 0) is never re-bounded to sndbuf_desc->len,
>writing user data past the send buffer -- a heap out-of-bounds write of
>attacker-influenced length and content.
Hi Bryam,
I think this is the same as patch #2.
Best regards,
Dust
>
>Bound the write space to sndbuf_desc->len where it is consumed, treating
>a negative (sign-overflowed) value as out of range too, so the copy
>length can never exceed the ring. This enforces the documented
>0 <= sndbuf_space <= sndbuf_desc->len invariant at the producer,
>race-free against the CDC tasklet that advances sndbuf_space.
>
>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>
>---
> 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.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers
2026-06-14 8:23 [PATCH v3 0/3] net/smc: bound wire-controlled CDC cursors against the local buffers Bryam Vargas via B4 Relay
` (2 preceding siblings ...)
2026-06-14 8:23 ` [PATCH v3 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg() Bryam Vargas via B4 Relay
@ 2026-06-17 23:24 ` Jakub Kicinski
3 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2026-06-17 23:24 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay
Cc: hexlabsecurity, Wenjia Zhang, Dust Li, D. Wythe, Sidraya Jayagond,
Eric Dumazet, David S. Miller, Mahanta Jambigi, Wen Gu,
Simon Horman, netdev, Ursula Braun, Stefan Raspl, linux-s390,
Paolo Abeni, linux-kernel, linux-rdma, Tony Lu
On Sun, 14 Jun 2026 03:23:29 -0500 Bryam Vargas via B4 Relay wrote:
> 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 of these runs past the relevant buffer:
> an out-of-bounds read of adjacent kernel memory (disclosed to the peer) on the
> receive/urgent side, and an out-of-bounds write of attacker-influenced length
> and content on the send side.
Once again, SMC maintainers -- please review.
--
mping: SHARED MEMORY COMMUNICATIONS (SMC) SOCKETS
^ permalink raw reply [flat|nested] 8+ messages in thread