All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it
@ 2026-07-20 17:07 Ibrahim Hashimov
  2026-07-21 17:08 ` sashiko-bot
  2026-07-22  3:47 ` Dust Li
  0 siblings, 2 replies; 4+ messages in thread
From: Ibrahim Hashimov @ 2026-07-20 17:07 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's count is
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; the wire cursor
is peer-controlled and was never checked against that.

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 new->count - old->count without clamping against size. A peer
that puts an out-of-range prod.count on the wire (e.g. 0x7fffffff) 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.

Fix it where the wire cursor is accepted, mirroring the cursor-sanity
idiom two lines above: reject any incoming cursor whose count is >= the
size of the buffer it will index. 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. This
bounds 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
---
 net/smc/smc_cdc.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 696cc11f2303..f07bbef47073 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,14 @@ 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;
 	smc_curs_copy(local, &temp, conn);
 }
 
@@ -246,8 +255,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] net/smc: validate peer CDC cursor against RMBE size before accepting it
  2026-07-20 17:07 [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it Ibrahim Hashimov
@ 2026-07-21 17:08 ` sashiko-bot
  2026-07-22  3:47 ` Dust Li
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-21 17:08 UTC (permalink / raw)
  To: Ibrahim Hashimov
  Cc: Heiko Carstens, linux-s390, Vasily Gorbik, Alexander Gordeev

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

Pre-existing issues:
- [Critical] The patch fails to prevent remote kernel memory disclosure because an attacker can bypass the bounds check by exploiting a flaw in the `wrap` counter validation, which allows infinite inflation of `bytes_to_rcv`.
- [High] The patch explicitly excludes SMC-D from cursor bounds checking, leaving it fully vulnerable to the original unbounded cursor out-of-bounds read and write attacks.
- [High] Incoming CDC messages processed during early connection establishment can trigger a NULL pointer dereference on `conn->rmb_desc->len`.
--

--- Patch [1]: [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it ---
commit 2f986d59395ef38131fb12b9fd2f5e861bf4a2d9
Author: Ibrahim Hashimov <security@auditcode.ai>

net/smc: validate peer CDC cursor against RMBE size before accepting it

This commit validates incoming CDC message cursors against the receiving
buffer size to prevent out-of-bounds reads/writes. It modifies
smc_cdc_cursor_to_host() and smcr_cdc_msg_to_host() to reject counts
exceeding the buffer length.

> SMC-D (smcd_cdc_msg_to_host()) does not use this helper and is
> a separate, out-of-scope gap.

[Severity: High]
This isn't a bug introduced by this patch, but does explicitly excluding
SMC-D leave it vulnerable to the same unbounded cursor out-of-bounds read
and write attacks?

If an untrusted SMC-D peer sends a CDC message with a massive cursor count
(e.g. 0x7FFFFFFF), smcd_cdc_msg_to_host() blindly copies it without bounds
checks. When smc_cdc_msg_recv_action() consumes it via smc_curs_diff(), it
massively inflates bytes_to_rcv.

Would it be safer to apply similar validation for SMC-D connections to
prevent arbitrary out-of-bounds reads in smc_rx_recvmsg()?

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

[ ... ]

> @@ -235,6 +236,14 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
>  	if ((old.wrap > temp.wrap) && temp.wrap)
>  		return;

[Severity: Critical]
This is a pre-existing issue, but does this wrap check flaw allow a malicious
peer to bypass the newly added bounds check entirely?

If a malicious SMC-R peer alternates sending CDC messages with wrap=1 and
wrap=0, the check above evaluates to false when temp.wrap is 0. This
improperly allows a backward wrap jump to be accepted.

Consequently, smc_curs_diff() will compute a massive positive difference
when the wraps differ, which is then unconditionally added to
conn->bytes_to_rcv in smc_cdc_msg_recv_action().

Could this inflation of bytes_to_rcv defeat the count bounds check and
still lead to remote out-of-bounds memory disclosure?

>  	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;
>  	smc_curs_copy(local, &temp, conn);
>  }

[ ... ]

> @@ -246,8 +255,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);

[Severity: High]
This isn't a bug introduced by this patch, but does directly dereferencing
conn->rmb_desc here expose a NULL pointer dereference during early connection
establishment?

In smc_conn_create(), a connection is made globally discoverable by adding
it to lgr->conns_all before the underlying buffers (like conn->rmb_desc)
are allocated:

smc_core.c:smc_conn_create() {
    ...
    write_lock_bh(&lgr->conns_lock);
    rc = smc_lgr_register_conn(conn, true);
    write_unlock_bh(&lgr->conns_lock);
    ...
}

If an attacker predicts the sequential atomic token and sends a CDC message
during this race window, smc_cdc_rx_handler() finds the connection and
proceeds to message processing.

Would this result in a kernel panic when smcr_cdc_msg_to_host() evaluates
conn->rmb_desc->len before the buffer is initialized?

> +	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;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720170717.11425-1-security@auditcode.ai?part=1

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

* Re: [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it
  2026-07-20 17:07 [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it Ibrahim Hashimov
  2026-07-21 17:08 ` sashiko-bot
@ 2026-07-22  3:47 ` Dust Li
  2026-07-22 10:29   ` Ibrahim Hashimov
  1 sibling, 1 reply; 4+ messages in thread
From: Dust Li @ 2026-07-22  3:47 UTC (permalink / raw)
  To: Ibrahim Hashimov, alibuda, wenjia, davem, edumazet, kuba, pabeni
  Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
	linux-kernel, stable

On 2026-07-20 19:07:17, Ibrahim Hashimov wrote:

Hi Ibrahim,

I believe Bryam already sent a similar patch to the mailist and I have already
reviewed it.

https://lore.kernel.org/netdev/20260705-b4-disp-28a1bbca-v4-1-be089b98acc6@proton.me/

Best regards,
Dust


>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's count is
>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; the wire cursor
>is peer-controlled and was never checked against that.
>
>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 new->count - old->count without clamping against size. A peer
>that puts an out-of-range prod.count on the wire (e.g. 0x7fffffff) 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.
>
>Fix it where the wire cursor is accepted, mirroring the cursor-sanity
>idiom two lines above: reject any incoming cursor whose count is >= the
>size of the buffer it will index. 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. This
>bounds 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
>---
> net/smc/smc_cdc.h | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
>index 696cc11f2303..f07bbef47073 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,14 @@ 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;
> 	smc_curs_copy(local, &temp, conn);
> }
> 
>@@ -246,8 +255,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] net/smc: validate peer CDC cursor against RMBE size before accepting it
  2026-07-22  3:47 ` Dust Li
@ 2026-07-22 10:29   ` Ibrahim Hashimov
  0 siblings, 0 replies; 4+ messages in thread
From: Ibrahim Hashimov @ 2026-07-22 10:29 UTC (permalink / raw)
  To: dust.li
  Cc: alibuda, wenjia, hexlabsecurity, tonylu, guwen, netdev,
	linux-rdma, linux-s390, linux-kernel

> I believe Bryam already sent a similar patch to the mailist and I have
> already reviewed it.
> https://lore.kernel.org/netdev/20260705-b4-disp-28a1bbca-v4-1-be089b98acc6@proton.me/

Thanks Dust -- yes, that fixes the same bug, so please drop mine. (I sent
a v2 earlier today, before I saw your note, in reply to a sashiko review;
please disregard it as a competing patch.)

One thing worth checking on Bryam's series, though: bounding only the cursor
count (clamping temp.count to rmb_desc->len) still leaves the advance
unbounded. smc_curs_diff() on a wrap increment returns
(size - old.count) + new.count, so a peer that sends prod (wrap=W, count=0)
then prod (wrap=W+1, count=size) makes diff_prod ~= 2*size even though both
counts are in range. smc_cdc_msg_recv_action() then atomic_add()s that into
bytes_to_rcv without clamping (despite the "0 <= bytes_to_rcv <=
rmb_desc->len" comment) -- which is exactly what smc_rx_recvmsg()'s second
copy chunk trusts.

A sashiko review of my patch flagged the same gap; I bounded the advance
too, with:

	if (smc_curs_diff(size, &old, &temp) > size)
		return;

Might be worth folding into Bryam's version. Happy to send it as a
follow-up if that helps.

Thanks,
Ibrahim

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

end of thread, other threads:[~2026-07-22 10:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 17:07 [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it Ibrahim Hashimov
2026-07-21 17:08 ` sashiko-bot
2026-07-22  3:47 ` Dust Li
2026-07-22 10:29   ` Ibrahim Hashimov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.