Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it
@ 2026-07-22 10:17 Ibrahim Hashimov
  2026-07-28 10:04 ` Paolo Abeni
  2026-07-28 11:34 ` D. Wythe
  0 siblings, 2 replies; 4+ messages in thread
From: Ibrahim Hashimov @ 2026-07-22 10:17 UTC (permalink / raw)
  To: alibuda, dust.li, wenjia, davem, edumazet, kuba, pabeni
  Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
	linux-kernel, stable

smc_cdc_cursor_to_host() converts the wire-format producer/consumer
cursor of an incoming CDC message into a host smc_host_cursor. It rejects
a cursor that goes backwards, but never checks that the cursor stays
inside the buffer it indexes. Per smc_host_cursor ("an offset in an
RMBE") and the invariant smc_curs_add() enforces for every local advance
(0 <= count < size), a valid cursor count must be < size and a single
advance can be at most one bufferful; the wire cursor is peer-controlled
and was never checked against either.

smcr_cdc_msg_to_host() accepts the producer and consumer cursors, and the
unbounded value feeds smc_curs_diff() in smc_cdc_msg_recv_action(), which
computes the advance without clamping against size. A peer can inflate it
two ways: an out-of-range prod.count (e.g. 0x7fffffff), or -- since on a
wrap increment smc_curs_diff() returns (size - old.count) + new.count --
a cursor with a bumped wrap and new.count above old.count. Either drives
bytes_to_rcv far past rmb_desc->len, the very invariant the comment above
the atomic_add() claims but does not enforce.

smc_rx_recvmsg() then trusts bytes_to_rcv as the amount of valid RMB
data. Its first copy chunk is safely bounded by rmb_desc->len -
cons.count, but the second chunk copies (copylen - chunk_len) bytes from
offset 0, and copylen came from the inflated readable -- an out-of-bounds
read of the RMB's backing (v)malloc allocation, copied straight to the
receiving process via _copy_to_iter(). This is a remote kernel-memory
disclosure driven by a malicious SMC-R peer, needing no local privilege
on the victim. The same unbounded count also reaches
smc_cdc_handle_urg_data_arrival() (base + urg_curs.count - 1) -- a
second, narrower OOB read.

Reject the bad cursor at ingress, mirroring the cursor-sanity idiom two
lines above: drop any incoming cursor whose count is outside the buffer
(>= size), and any whose advance from the last accepted cursor exceeds
one bufferful (smc_curs_diff() > size), which catches the wrap case.
smc_cdc_cursor_to_host() gains a size parameter; smcr_cdc_msg_to_host()
passes conn->rmb_desc->len for the producer cursor and
conn->peer_rmbe_size for the consumer cursor, bounding it at its single
origin instead of patching every downstream consumer. SMC-D
(smcd_cdc_msg_to_host()) does not use this helper and is a separate,
out-of-scope gap.

Reproduced on a v6.19 KASAN kernel (SMC-Rv2 over soft-RoCE): a peer
emitting prod.count=0x7fffffff triggers a vmalloc-out-of-bounds read in
_copy_to_iter() via smc_rx_recvmsg(), returning ~960 KB of RMB-adjacent
kernel heap to userspace; an in-range cursor transfers cleanly. With the
fix the out-of-range cursor is dropped before it reaches
conn->local_rx_ctrl.prod, and the reproducer returns rc=0 with no report.

Fixes: 5f08318f617b ("smc: connection data control (CDC)")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: also bound the cursor *advance* (smc_curs_diff() <= size), not just
    the count. A wrap increment with new.count above old.count passes the
    count check yet still inflates bytes_to_rcv past the buffer -- thanks
    to sashiko-bot for spotting the count-only gap. Resent standalone.

 net/smc/smc_cdc.h | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 696cc11f2303..b0ae4d43b017 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h
@@ -221,6 +221,7 @@ 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,
+					  unsigned int size,
 					  struct smc_connection *conn)
 {
 	union smc_host_cursor temp, old;
@@ -235,6 +236,21 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
 	if ((old.wrap == temp.wrap) &&
 	    (old.count > temp.count))
 		return;
+	/* count is an offset into the RMBE and must always stay inside
+	 * it (see smc_curs_add()); the peer is untrusted, so reject an
+	 * out-of-range wire cursor the same way an out-of-order one is
+	 * already rejected above, instead of letting it drive
+	 * bytes_to_rcv / the urgent-byte offset past the buffer end
+	 */
+	if (temp.count >= size)
+		return;
+	/* Bound the advance too: a wrap increment with temp.count above
+	 * old.count makes smc_curs_diff() report more than one bufferful,
+	 * which would inflate bytes_to_rcv / peer_rmbe_space past the buffer
+	 * in smc_cdc_msg_recv_action(). Reject such a cursor.
+	 */
+	if (smc_curs_diff(size, &old, &temp) > size)
+		return;
 	smc_curs_copy(local, &temp, conn);
 }
 
@@ -246,8 +262,10 @@ 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->rmb_desc->len, conn);
+	smc_cdc_cursor_to_host(&local->cons, &peer->cons,
+			       conn->peer_rmbe_size, conn);
 	local->prod_flags = peer->prod_flags;
 	local->conn_state_flags = peer->conn_state_flags;
 }
-- 
2.50.1 (Apple Git-155)

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

* Re: [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it
  2026-07-22 10:17 [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it Ibrahim Hashimov
@ 2026-07-28 10:04 ` Paolo Abeni
  2026-07-28 11:37   ` Dust Li
  2026-07-28 11:34 ` D. Wythe
  1 sibling, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2026-07-28 10:04 UTC (permalink / raw)
  To: Ibrahim Hashimov, alibuda, dust.li, wenjia, davem, edumazet, kuba
  Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
	linux-kernel, stable

On 7/22/26 12:17 PM, Ibrahim Hashimov wrote:
> smc_cdc_cursor_to_host() converts the wire-format producer/consumer
> cursor of an incoming CDC message into a host smc_host_cursor. It rejects
> a cursor that goes backwards, but never checks that the cursor stays
> inside the buffer it indexes. Per smc_host_cursor ("an offset in an
> RMBE") and the invariant smc_curs_add() enforces for every local advance
> (0 <= count < size), a valid cursor count must be < size and a single
> advance can be at most one bufferful; the wire cursor is peer-controlled
> and was never checked against either.
> 
> smcr_cdc_msg_to_host() accepts the producer and consumer cursors, and the
> unbounded value feeds smc_curs_diff() in smc_cdc_msg_recv_action(), which
> computes the advance without clamping against size. A peer can inflate it
> two ways: an out-of-range prod.count (e.g. 0x7fffffff), or -- since on a
> wrap increment smc_curs_diff() returns (size - old.count) + new.count --
> a cursor with a bumped wrap and new.count above old.count. Either drives
> bytes_to_rcv far past rmb_desc->len, the very invariant the comment above
> the atomic_add() claims but does not enforce.
> 
> smc_rx_recvmsg() then trusts bytes_to_rcv as the amount of valid RMB
> data. Its first copy chunk is safely bounded by rmb_desc->len -
> cons.count, but the second chunk copies (copylen - chunk_len) bytes from
> offset 0, and copylen came from the inflated readable -- an out-of-bounds
> read of the RMB's backing (v)malloc allocation, copied straight to the
> receiving process via _copy_to_iter(). This is a remote kernel-memory
> disclosure driven by a malicious SMC-R peer, needing no local privilege
> on the victim. The same unbounded count also reaches
> smc_cdc_handle_urg_data_arrival() (base + urg_curs.count - 1) -- a
> second, narrower OOB read.
> 
> Reject the bad cursor at ingress, mirroring the cursor-sanity idiom two
> lines above: drop any incoming cursor whose count is outside the buffer
> (>= size), and any whose advance from the last accepted cursor exceeds
> one bufferful (smc_curs_diff() > size), which catches the wrap case.
> smc_cdc_cursor_to_host() gains a size parameter; smcr_cdc_msg_to_host()
> passes conn->rmb_desc->len for the producer cursor and
> conn->peer_rmbe_size for the consumer cursor, bounding it at its single
> origin instead of patching every downstream consumer. SMC-D
> (smcd_cdc_msg_to_host()) does not use this helper and is a separate,
> out-of-scope gap.

@SMC crew: please review.

The above looks so correlated and quite simple to address that possibly
it makes sense to address it in the same series.

/P


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

* Re: [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it
  2026-07-22 10:17 [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it Ibrahim Hashimov
  2026-07-28 10:04 ` Paolo Abeni
@ 2026-07-28 11:34 ` D. Wythe
  1 sibling, 0 replies; 4+ messages in thread
From: D. Wythe @ 2026-07-28 11:34 UTC (permalink / raw)
  To: Ibrahim Hashimov
  Cc: alibuda, dust.li, wenjia, davem, edumazet, kuba, pabeni, tonylu,
	guwen, horms, linux-rdma, linux-s390, netdev, linux-kernel,
	stable

On Wed, Jul 22, 2026 at 12:17:58PM +0200, Ibrahim Hashimov wrote:
> smc_cdc_cursor_to_host() converts the wire-format producer/consumer
> cursor of an incoming CDC message into a host smc_host_cursor. It rejects
> a cursor that goes backwards, but never checks that the cursor stays
> inside the buffer it indexes. Per smc_host_cursor ("an offset in an
> RMBE") and the invariant smc_curs_add() enforces for every local advance
> (0 <= count < size), a valid cursor count must be < size and a single
> advance can be at most one bufferful; the wire cursor is peer-controlled
> and was never checked against either.
> 
> smcr_cdc_msg_to_host() accepts the producer and consumer cursors, and the
> unbounded value feeds smc_curs_diff() in smc_cdc_msg_recv_action(), which
> computes the advance without clamping against size. A peer can inflate it
> two ways: an out-of-range prod.count (e.g. 0x7fffffff), or -- since on a
> wrap increment smc_curs_diff() returns (size - old.count) + new.count --
> a cursor with a bumped wrap and new.count above old.count. Either drives
> bytes_to_rcv far past rmb_desc->len, the very invariant the comment above
> the atomic_add() claims but does not enforce.
> 
> smc_rx_recvmsg() then trusts bytes_to_rcv as the amount of valid RMB
> data. Its first copy chunk is safely bounded by rmb_desc->len -
> cons.count, but the second chunk copies (copylen - chunk_len) bytes from
> offset 0, and copylen came from the inflated readable -- an out-of-bounds
> read of the RMB's backing (v)malloc allocation, copied straight to the
> receiving process via _copy_to_iter(). This is a remote kernel-memory
> disclosure driven by a malicious SMC-R peer, needing no local privilege
> on the victim. The same unbounded count also reaches
> smc_cdc_handle_urg_data_arrival() (base + urg_curs.count - 1) -- a
> second, narrower OOB read.
> 
> Reject the bad cursor at ingress, mirroring the cursor-sanity idiom two
> lines above: drop any incoming cursor whose count is outside the buffer
> (>= size), and any whose advance from the last accepted cursor exceeds
> one bufferful (smc_curs_diff() > size), which catches the wrap case.
> smc_cdc_cursor_to_host() gains a size parameter; smcr_cdc_msg_to_host()
> passes conn->rmb_desc->len for the producer cursor and
> conn->peer_rmbe_size for the consumer cursor, bounding it at its single
> origin instead of patching every downstream consumer. SMC-D
> (smcd_cdc_msg_to_host()) does not use this helper and is a separate,
> out-of-scope gap.
> 
> Reproduced on a v6.19 KASAN kernel (SMC-Rv2 over soft-RoCE): a peer
> emitting prod.count=0x7fffffff triggers a vmalloc-out-of-bounds read in
> _copy_to_iter() via smc_rx_recvmsg(), returning ~960 KB of RMB-adjacent
> kernel heap to userspace; an in-range cursor transfers cleanly. With the
> fix the out-of-range cursor is dropped before it reaches
> conn->local_rx_ctrl.prod, and the reproducer returns rc=0 with no report.
> 
> Fixes: 5f08318f617b ("smc: connection data control (CDC)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07
> ---
> v2: also bound the cursor *advance* (smc_curs_diff() <= size), not just
>     the count. A wrap increment with new.count above old.count passes the
>     count check yet still inflates bytes_to_rcv past the buffer -- thanks
>     to sashiko-bot for spotting the count-only gap. Resent standalone.
> 
>  net/smc/smc_cdc.h | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
> index 696cc11f2303..b0ae4d43b017 100644
> --- a/net/smc/smc_cdc.h
> +++ b/net/smc/smc_cdc.h
> @@ -221,6 +221,7 @@ 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,
> +					  unsigned int size,
>  					  struct smc_connection *conn)
>  {
>  	union smc_host_cursor temp, old;
> @@ -235,6 +236,21 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
>  	if ((old.wrap == temp.wrap) &&
>  	    (old.count > temp.count))
>  		return;
> +	/* count is an offset into the RMBE and must always stay inside
> +	 * it (see smc_curs_add()); the peer is untrusted, so reject an
> +	 * out-of-range wire cursor the same way an out-of-order one is
> +	 * already rejected above, instead of letting it drive
> +	 * bytes_to_rcv / the urgent-byte offset past the buffer end
> +	 */
> +	if (temp.count >= size)
> +		return;
> +	/* Bound the advance too: a wrap increment with temp.count above
> +	 * old.count makes smc_curs_diff() report more than one bufferful,
> +	 * which would inflate bytes_to_rcv / peer_rmbe_space past the buffer
> +	 * in smc_cdc_msg_recv_action(). Reject such a cursor.
> +	 */
> +	if (smc_curs_diff(size, &old, &temp) > size)
> +		return;


At minimum there should be a rate-limited log indicating something
unusual happened. Ideally the connection should be actively aborted,
a peer sending an out-of-range cursor is either malicious, buggy, or the
data is corrupted, and silently continuing is pointless.


>  	smc_curs_copy(local, &temp, conn);
>  }
>  
> @@ -246,8 +262,10 @@ 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->rmb_desc->len, conn);
> +	smc_cdc_cursor_to_host(&local->cons, &peer->cons,
> +			       conn->peer_rmbe_size, conn);
>  	local->prod_flags = peer->prod_flags;
>  	local->conn_state_flags = peer->conn_state_flags;
>  }
> -- 
> 2.50.1 (Apple Git-155)

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

* Re: [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it
  2026-07-28 10:04 ` Paolo Abeni
@ 2026-07-28 11:37   ` Dust Li
  0 siblings, 0 replies; 4+ messages in thread
From: Dust Li @ 2026-07-28 11:37 UTC (permalink / raw)
  To: Paolo Abeni, Ibrahim Hashimov, alibuda, wenjia, davem, edumazet,
	kuba
  Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
	linux-kernel, stable

On 2026-07-28 12:04:32, Paolo Abeni wrote:
>On 7/22/26 12:17 PM, Ibrahim Hashimov wrote:
>> smc_cdc_cursor_to_host() converts the wire-format producer/consumer
>> cursor of an incoming CDC message into a host smc_host_cursor. It rejects
>> a cursor that goes backwards, but never checks that the cursor stays
>> inside the buffer it indexes. Per smc_host_cursor ("an offset in an
>> RMBE") and the invariant smc_curs_add() enforces for every local advance
>> (0 <= count < size), a valid cursor count must be < size and a single
>> advance can be at most one bufferful; the wire cursor is peer-controlled
>> and was never checked against either.
>> 
>> smcr_cdc_msg_to_host() accepts the producer and consumer cursors, and the
>> unbounded value feeds smc_curs_diff() in smc_cdc_msg_recv_action(), which
>> computes the advance without clamping against size. A peer can inflate it
>> two ways: an out-of-range prod.count (e.g. 0x7fffffff), or -- since on a
>> wrap increment smc_curs_diff() returns (size - old.count) + new.count --
>> a cursor with a bumped wrap and new.count above old.count. Either drives
>> bytes_to_rcv far past rmb_desc->len, the very invariant the comment above
>> the atomic_add() claims but does not enforce.
>> 
>> smc_rx_recvmsg() then trusts bytes_to_rcv as the amount of valid RMB
>> data. Its first copy chunk is safely bounded by rmb_desc->len -
>> cons.count, but the second chunk copies (copylen - chunk_len) bytes from
>> offset 0, and copylen came from the inflated readable -- an out-of-bounds
>> read of the RMB's backing (v)malloc allocation, copied straight to the
>> receiving process via _copy_to_iter(). This is a remote kernel-memory
>> disclosure driven by a malicious SMC-R peer, needing no local privilege
>> on the victim. The same unbounded count also reaches
>> smc_cdc_handle_urg_data_arrival() (base + urg_curs.count - 1) -- a
>> second, narrower OOB read.
>> 
>> Reject the bad cursor at ingress, mirroring the cursor-sanity idiom two
>> lines above: drop any incoming cursor whose count is outside the buffer
>> (>= size), and any whose advance from the last accepted cursor exceeds
>> one bufferful (smc_curs_diff() > size), which catches the wrap case.
>> smc_cdc_cursor_to_host() gains a size parameter; smcr_cdc_msg_to_host()
>> passes conn->rmb_desc->len for the producer cursor and
>> conn->peer_rmbe_size for the consumer cursor, bounding it at its single
>> origin instead of patching every downstream consumer. SMC-D
>> (smcd_cdc_msg_to_host()) does not use this helper and is a separate,
>> out-of-scope gap.
>
>@SMC crew: please review.
>
>The above looks so correlated and quite simple to address that possibly
>it makes sense to address it in the same series.

Hi Paolo,

This patch was discussed in the v1 thread. Bryan had already sent a the
v5 addressing the issue, so I think this patch can be dropped.

https://lore.kernel.org/netdev/20260723-b4-disp-0d07164f-v5-0-6a9e235dbc4e@proton.me

Best regards,
Dust

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

end of thread, other threads:[~2026-07-28 11:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22 10:17 [PATCH net v2] net/smc: validate peer CDC cursor against RMBE size before accepting it Ibrahim Hashimov
2026-07-28 10:04 ` Paolo Abeni
2026-07-28 11:37   ` Dust Li
2026-07-28 11:34 ` D. Wythe

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