Re: [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it

[sashiko-bot@kernel.org]

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