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

Simon Horman <[email protected]>
Newsgroups org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel,org.kernel.vger.linux-rdma,org.kernel.vger.linux-s390,org.kernel.vger.netdev
Message-ID <[email protected]>
On Sat, Aug 01, 2026 at 06:42:08PM +0900, Yehyeong Lee wrote:
> An LLC message longer than the 44-byte union smc_llc_msg arrives either
> whole in the receive buffer or split into the link group's shared v2
> buffer, and both consumers of the tail rebuild that layout themselves.
> Copy the tail into the queue entry instead, so its length and its lifetime
> are those of the message that arrived, and declare the rkeys that do fit
> inline as a member of the union rather than reaching them through a cast of
> a shorter type.
> 
> The bound the previous patch placed on links without a shared v2 receive
> buffer is no longer needed here: the extension is read from the qentry,
> whose length is the length of the message that arrived.
> 
> A DELETE_RKEY_V2 is now bounded by what arrived rather than by the buffer
> it landed in: a 44-byte message declaring 255 rkeys processed 255 of them
> before, and processes the 9 that fit now.
> 
> Suggested-by: D. Wythe <[email protected]>
> Signed-off-by: Yehyeong Lee <[email protected]>
> ---
> Measured over rxe with KASAN: a DELETE_RKEY_V2 carrying 12 rkeys over a link
> with a shared v2 receive buffer round-trips all 12 values, the last three
> coming from the copied tail; 8, 9 and 10 rkeys and a 44-byte message declaring
> 10 give 8, 9, 10 and 9 processed rkeys respectively.  kmemleak reports nothing
> over the link-addition path, and does report the queue entry when the free
> added by patch 1 is removed again.

There is an AI generated review of this patch-set available at
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260801094208.1937951-1-yhlee%40isslab.korea.ac.kr

Of that review, I think the following warrants particular consideration.

TL;DR: to ensure complete backports, maybe add this tag to this patch,
as it has been added to patch 2/3.

  Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")

The text of the AI-generated review:

  Should this patch carry a Fixes: tag and Cc: stable as well?  Both
  predecessors in the series do, and this is the patch that changes behaviour
  in two places.

  First, the early return this hunk replaces:

	if (!smc_link_shared_v2_rxbuf(link))
		return;
  On a device with max_recv_sge == 1 that means the peer's rkeys in the
  ADD_LINK v2 extension are never installed into lgr->rtokens for the new
  link, even though those bytes do arrive on the link.  Only the
  qentry->body_len bound added here reaches them.

  Second, in smc_llc_rmt_delete_rkey() the shared-v2 path was bounded by the
  capacity of the per-lgr buffer rather than by what the peer sent:

	llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
	buf_len = SMC_WR_BUF_V2_SIZE;
	...
	max = min_t(u32, max, (buf_len - rkey_off) / sizeof(llcv2->rkey[0]));

  So a 44-byte DELETE_RKEY_V2 declaring num_rkeys = 255 walks rkey[9..254] out
  of whatever an earlier oversized peer message left in lgr->wr_rx_buf_v2 and
  passes each value to smc_rtoken_delete(), which can clear a valid rtoken of
  an unrelated connection in the same link group.

  The changelog describes both as consequences rather than as fixes:

	The bound the previous patch placed on links without a shared v2
	receive buffer is no longer needed here
	A DELETE_RKEY_V2 is now bounded by what arrived rather than by the
	buffer it landed in: a 44-byte message declaring 255 rkeys processed
	255 of them before, and processes the 9 that fit now.

  If a stable tree takes the first two patches of the series but not this one,
  does it end up with the ADD_LINK v2 extension ignored on max_recv_sge == 1
  devices and the stale-rkey deletion still present?
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.