Re: [PATCH net v7 3/3] net/smc: carry oversized SMC-Rv2 LLC messages in the queue entry

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

Pre-existing issues:
- [High] The `num_rkeys` field in `struct smc_llc_msg_add_link_v2_ext` is transmitted in native byte order without endianness conversion, breaking cross-architecture SMC-Rv2 connections.
--

--- Patch [3]: [PATCH net v7 3/3] net/smc: carry oversized SMC-Rv2 LLC messages in the queue entry ---
commit 411251064d6efa68e4e9521d534e61850c251cba
Author: Yehyeong Lee <[email protected]>

net/smc: carry oversized SMC-Rv2 LLC messages in the queue entry

This patch modifies SMC-Rv2 LLC message handling to store oversized
messages in the queue entry instead of relying on shared receive buffers.
It dynamically allocates the qentry to carry the remainder of the message
tail.

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> index f04e0392ec96c..481b0495b0d31 100644
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c

[ ... ]

> @@ -998,22 +1019,19 @@ static int smc_llc_cli_conf_link(struct smc_link *link,
>  
>  static void smc_llc_save_add_link_rkeys(struct smc_link *link,
>  					struct smc_link *link_new,
> -					u8 *llc_msg)
> +					struct smc_llc_qentry *qentry)
>  {
>  	const u32 rt_off = offsetof(struct smc_llc_msg_add_link_v2_ext, rt);
>  	struct smc_llc_msg_add_link_v2_ext *ext;
>  	struct smc_link_group *lgr = link->lgr;
>  	int max, i;
>  
> -	/* Without a shared v2 receive buffer the extension is not copied
> -	 * into the queue entry, so not even ext->num_rkeys is there.
> -	 */
> -	if (!smc_link_shared_v2_rxbuf(link))
> +	/* the rkey count itself is only there if enough bytes arrived */
> +	if (qentry->body_len < rt_off)
>  		return;
> -	ext = (struct smc_llc_msg_add_link_v2_ext *)(llc_msg +
> -						     SMC_WR_TX_SIZE);
> +	ext = (struct smc_llc_msg_add_link_v2_ext *)qentry->body;
>  	max = min_t(u8, ext->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);

[Severity: High]
This is a pre-existing issue, but does this code handle endianness
correctly across different architectures?

In smc_llc_save_add_link_rkeys(), ext->num_rkeys is read from the wire.
If an SMC connection is established between a big-endian (like s390x) and
a little-endian (like x86) host, a value of 1 could be interpreted as 256.

When passed to min_t(u8, ...), the cast to u8 truncates 256 to 0, which
would cause the receiver to skip processing any rkeys and silently drop
them.

Should there be an ntohs() conversion here to ensure the number of rkeys
is safely parsed on cross-architecture setups?

> -	max = min_t(u32, max, (SMC_WR_BUF_V2_SIZE - SMC_WR_TX_SIZE - rt_off) /
> +	max = min_t(u32, max, (qentry->body_len - rt_off) /
>  			      sizeof(ext->rt[0]));
>  	down_write(&lgr->rmbs_lock);
>  	for (i = 0; i < max; i++) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3