Re: [PATCH net] net/smc: ignore peer-supplied rmbe_idx and dmbe_idx

[sashiko-bot@kernel.org]

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

New issues:
- [High] Hardcoding `peer_rmbe_idx` and `tx_off` violates the SMC protocol and breaks interoperability by ignoring the peer-supplied buffer index.

Pre-existing issues:
- [Critical] Missing bounds check in `dibs_lo_move_data` allows an arbitrary out-of-bounds write in the kernel.
--

commit 60af5c80d503d1904779918ca5eeba8800e8bbe9
Author: Dust Li <dust.li@linux.alibaba.com>

net/smc: ignore peer-supplied rmbe_idx and dmbe_idx

This commit modifies the smc connection setup to ignore the peer-supplied
rmbe and dmbe indices from the clc accept/confirm messages. It hardcodes
the peer_rmbe_idx to 1 for smc-r and 0 for smc-d, and unconditionally sets
tx_off to 0, assuming the peer allocates exactly one buffer.

> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -729,11 +729,15 @@ static void smcr_conn_save_peer_info(struct smc_sock *smc,
>  {
>  	int bufsize = smc_uncompress_bufsize(clc->r0.rmbe_size);
>  
> -	smc->conn.peer_rmbe_idx = clc->r0.rmbe_idx;
> +	/* Linux uses exactly one RMBE per RMB (always index 1); ignore the
> +	 * peer-supplied rmbe_idx to prevent a malicious peer from setting an
> +	 * out-of-bounds tx_off.
> +	 */
> +	smc->conn.peer_rmbe_idx = 1;
>  	smc->conn.local_tx_ctrl.token = ntohl(clc->r0.rmbe_alert_token);
>  	smc->conn.peer_rmbe_size = bufsize;
>  	atomic_set(&smc->conn.peer_rmbe_space, smc->conn.peer_rmbe_size);
> -	smc->conn.tx_off = bufsize * (smc->conn.peer_rmbe_idx - 1);
> +	smc->conn.tx_off = 0;

[Severity: High]
Does hardcoding the peer_rmbe_idx and tx_off violate the smc protocol and
break interoperability with standard-compliant peers?

If a remote peer multiplexes multiple connections into a single registered
memory buffer and assigns an index greater than 1, ignoring this index could
cause the Linux client to write its rdma payloads to the wrong remote offset.

Could this corrupt the memory of other connections on the remote peer?

>  }
>  
>  static void smcd_conn_save_peer_info(struct smc_sock *smc,
> @@ -741,12 +745,16 @@ static void smcd_conn_save_peer_info(struct smc_sock *smc,
>  {
>  	int bufsize = smc_uncompress_bufsize(clc->d0.dmbe_size);
>  
> -	smc->conn.peer_rmbe_idx = clc->d0.dmbe_idx;
> +	/* Linux uses exactly one DMBE per DMB (always index 0); ignore the
> +	 * peer-supplied dmbe_idx to prevent a malicious peer from deriving an
> +	 * out-of-bounds tx_off that causes an OOB write.
> +	 */
> +	smc->conn.peer_rmbe_idx = 0;
>  	smc->conn.peer_token = ntohll(clc->d0.token);
>  	/* msg header takes up space in the buffer */
>  	smc->conn.peer_rmbe_size = bufsize - sizeof(struct smcd_cdc_msg);
>  	atomic_set(&smc->conn.peer_rmbe_space, smc->conn.peer_rmbe_size);
> -	smc->conn.tx_off = bufsize * smc->conn.peer_rmbe_idx;
> +	smc->conn.tx_off = 0;

[Severity: High]
Similarly for smc-d, does discarding the peer-supplied dmbe_idx cause ism
writes to target the wrong remote offset if the remote peer uses an index
other than 0?

[Severity: Critical]
This isn't a bug introduced by this patch, but there appears to be a missing
bounds check in the smc loopback driver:

drivers/dibs/dibs_loopback.c:dibs_lo_move_data() {
    ...
    memcpy((char *)rmb_node->cpu_addr + offset, data, size);
    ...
}

If a malicious local process acting as an smc loopback peer allocates a
small dmb buffer but sends a forged clc accept message over the tcp socket
with a deceptively large dmbe_size, could this trigger an out-of-bounds
write?

The local smc client would compute a massive peer_rmbe_size and pass a large
size and offset to smcd->ops->move_data() when transmitting data. Without
verifying that the requested offset + size fits within the actually
allocated rmb_node->len, it seems this might overwrite kernel memory.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702171137.1099051-2-dust.li@linux.alibaba.com?part=1