Re: [PATCH net v2] net/smc: fix out-of-bounds read of rkey array in SMC-Rv2 LLC processing

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Data race and silent data corruption of the shared LLC receive buffer (`wr_rx_buf_v2`)
--

--- Patch [1]: [PATCH net v2] net/smc: fix out-of-bounds read of rkey array in SMC-Rv2 LLC processing ---
commit 60a54b5c8c2f6d12513324a22935be7ae509adaa
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

net/smc: fix out-of-bounds read of rkey array in SMC-Rv2 LLC processing

This commit fixes an out-of-bounds read in SMC-Rv2 LLC processing for devices
with max_recv_sge == 1 by preserving the message body in the link-group's
shared wr_rx_buf_v2 buffer, mirroring the behavior of the shared layout.

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> index 954b2ff1815c2..84d8d9024efa3 100644
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c

[ ... ]

> @@ -2104,6 +2105,26 @@ static void smc_llc_rx_handler(struct ib_wc *wc, void *buf)
>  	} else {
>  		if (llc->raw.hdr.length_v2 < sizeof(*llc))
>  			return; /* invalid message */
> +		/* For the non-shared v2 rxbuf layout the message body beyond
> +		 * SMC_WR_TX_SIZE lives only in the per-WR receive buffer, which
> +		 * is reposted as soon as this handler returns.  The LLC event
> +		 * handlers run later from a work item and read the body from
> +		 * wr_rx_buf_v2, so copy it there now - only the body, mirroring
> +		 * the shared-rxbuf layout where the spillover SGE DMAs the body
> +		 * into wr_rx_buf_v2 + SMC_WR_TX_SIZE.  The header is left to the
> +		 * handlers, which restore it from the per-qentry copy, so a later
> +		 * message reaching wr_rx_buf_v2 cannot clobber it.  wr_rx_buf_v2
> +		 * only exists for SMC_V2 link groups, so the link-group version
> +		 * is checked (not just the wire llc_version) to avoid a NULL
> +		 * dereference on a v1 link group.
> +		 */
> +		if (link->lgr->smc_version == SMC_V2 &&
> +		    !smc_link_shared_v2_rxbuf(link) &&
> +		    wc->byte_len > SMC_WR_TX_SIZE)
> +			memcpy((u8 *)link->lgr->wr_rx_buf_v2 + SMC_WR_TX_SIZE,
> +			       (u8 *)llc + SMC_WR_TX_SIZE,
> +			       min_t(u32, wc->byte_len, SMC_WR_BUF_V2_SIZE) -
> +			       SMC_WR_TX_SIZE);

[Severity: High]
This is a pre-existing issue, but does this pattern lead to silent data
corruption if multiple LLC messages arrive concurrently or back-to-back?

If local and remote DELETE_RKEY flows occur concurrently, the NIC might
receive multiple messages in quick succession. Since this memcpy (and the
equivalent hardware DMA in the shared layout) unconditionally writes to the
same link-group shared lgr->wr_rx_buf_v2, a second message will overwrite
the payload of the first.

When the background worker thread later processes the first message:

smc_llc_event_work()
  -> smc_llc_event_handler()
    -> smc_llc_rmt_delete_rkey() {
       llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
       ...
       if (smc_rtoken_delete(link, llcv2->rkey[i]))
    }
    
Will it end up reading the clobbered payload of the second message from
wr_rx_buf_v2, potentially deleting the wrong rkeys?

>  	}
>  
>  	smc_llc_enqueue(link, llc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724045310.430202-1-yhlee@isslab.korea.ac.kr?part=1