Re: [PATCH net v4 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] `num_rkeys` in `struct smc_llc_msg_add_link_v2_ext` is treated as a host-byte-order `u16`, causing interoperability failures between hosts of different endianness.
--

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

This commit copies the tail of an oversized SMC-Rv2 LLC message into the queue
entry instead of relying on the receiver to reconstruct it from the shared v2
buffer. It bounds DELETE_RKEY_V2 processing based on the arrived message length
instead of the buffer size.

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> index 606d2dc8c7ece..1b153173dd516 100644
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c

[ ... ]

> @@ -998,23 +1017,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;
>  
> -	/* The extension follows the 44 bytes of the LLC message.  Without a
> -	 * shared v2 receive buffer nothing beyond those 44 bytes was received,
> -	 * so not even ext->num_rkeys is there to be read.
> -	 */
> -	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 since this code is being modified, is it
safe to treat ext->num_rkeys as a host-byte-order u16 here?

If an x86 machine communicates with an s390x machine, smc_llc_fill_ext_v2()
writes lgr->conns_num into ext->num_rkeys in host byte order. A big-endian
receiver would read a value like 256.

When min_t() casts 256 to u8, wouldn't this truncate it to 0 and cause the
rkey exchange to fail on cross-endian links?

> -	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