[PATCH net v6 0/3] net/smc: fix out-of-bounds and use-after-free in SMC-Rv2 LLC processing

[PATCH net v6 0/3] net/smc: fix out-of-bounds and use-after-free in SMC-Rv2 LLC processing

[Yehyeong Lee]

Patch 1 fixes a use-after-free of the LLC queue entry in
smc_llc_srv_add_link(), patch 2 bounds the peer's rkey counts, and patch 3
carries the tail of an oversized v2 message in the queue entry so that both
readers are bounded by what arrived.  All three are tagged for stable: a
tree that takes 1 and 2 without 3 still deletes rkeys read from whatever an
earlier message left in the shared receive buffer.

Changes since v5:
 - 1/3: leave through the existing exit label instead of repeating the two
   kfree()s (Breno Leitao).  The object code is unchanged.
 - 3/3: add the Fixes: and Cc: stable tags (Simon Horman).
 - 3/3: assert that the two DELETE_RKEY_V2 layouts agree on the offset of
   rkey[], since the parsing code indexes one and the existing assert
   constrains the other.
 - 3/3: limit the copied tail to the largest one either reader can use,
   so the size of the queue entry is not chosen by the peer.
 - 3/3: the comment in smc_wr_init_sge() described the memcpy() this patch
   removes; correct it.

Yehyeong Lee (3):
  net/smc: fix use-after-free of the LLC qentry in
    smc_llc_srv_add_link()
  net/smc: bound the peer rkey counts in SMC-Rv2 LLC messages
  net/smc: carry oversized SMC-Rv2 LLC messages in the queue entry

---
v5: https://lore.kernel.org/netdev/20260801094208.1937951-1-yhlee@isslab.korea.ac.kr/

 net/smc/smc_llc.c | 122 ++++++++++++++++++++++++++++++++++++----------
 net/smc/smc_wr.c  |   6 +--
 2 files changed, 98 insertions(+), 30 deletions(-)

-- 
2.43.0

[PATCH net v6 1/3] net/smc: fix use-after-free of the LLC qentry in smc_llc_srv_add_link()

[Yehyeong Lee]

smc_llc_srv_add_link() keeps add_llc pointing into the queue entry:

  add_llc = &qentry->msg.add_link;			smc_llc.c:1482
  ...
  smc_llc_save_add_link_info(link_new, add_llc);	smc_llc.c:1494
  smc_llc_flow_qentry_del(&lgr->llc_flow_lcl);		smc_llc.c:1495
  ...
  u8 *llc_msg = smc_link_shared_v2_rxbuf(link) ?
	(u8 *)lgr->wr_rx_buf_v2 : (u8 *)add_llc;	smc_llc.c:1504
  smc_llc_save_add_link_rkeys(link, link_new, llc_msg);	smc_llc.c:1506

smc_llc_flow_qentry_del() kfree()s the entry, so on a link without a shared
v2 receive buffer the pointer handed to smc_llc_save_add_link_rkeys() is
already freed.  Before the Fixes: commit that branch always used
lgr->wr_rx_buf_v2 and add_llc was not used after the free.

Reproduced on an unpatched tree over rxe, with KASAN, kasan_multi_shot
and a link forced to max_recv_sge == 1: the entry is freed and read by
the same call, and the freeing frame is smc_llc_srv_add_link() itself.

  [    2.523161] BUG: KASAN: slab-use-after-free in smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.523499] Read of size 2 at addr ffff8880052194de by task kworker/0:1/11
  [    2.523789] 
  [    2.523862] CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Not tainted 7.2.0-rc5-p0-g2c9dd296545d #35 PREEMPT(lazy) 
  [    2.523865] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
  [    2.523866] Workqueue: smc_hs_wq smc_listen_work
  [    2.523869] Call Trace:
  [    2.523870]  <TASK>
  [    2.523871]  dump_stack_lvl+0x53/0x70
  [    2.523872]  print_report+0xd0/0x630
  [    2.523874]  ? __pfx__raw_spin_lock_irqsave+0x10/0x10
  [    2.523876]  ? smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.523878]  kasan_report+0xce/0x100
  [    2.523879]  ? smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.523881]  smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.523883]  ? smcr_buf_reg_lgr+0x2a4/0x660
  [    2.523885]  smc_llc_srv_add_link+0xaa2/0x1e50
  [    2.523888]  ? _printk+0xba/0xf0
  [    2.523897]  ? __pfx_smc_llc_srv_add_link+0x10/0x10
  [    2.523899]  ? down_write+0xb0/0x130
  [    2.523903]  ? __pfx_down_write+0x10/0x10
  [    2.523905]  smc_listen_work+0x489e/0x4d00
  [    2.523907]  ? kmem_cache_free+0x1c6/0x3a0
  [    2.523911]  ? __pfx_smc_listen_work+0x10/0x10
  [    2.523913]  ? release_sock+0x148/0x1d0
  [    2.523915]  ? smc_tcp_listen_work+0xb4f/0xfc0
  [    2.523917]  ? _raw_spin_lock_irq+0x80/0xe0
  [    2.523918]  ? __pfx__raw_spin_lock_irq+0x10/0x10
  [    2.523920]  process_one_work+0x633/0x1030
  [    2.523922]  ? assign_work+0x11d/0x370
  [    2.523924]  worker_thread+0x45b/0xd10
  [    2.523926]  ? __pfx_worker_thread+0x10/0x10
  [    2.523928]  ? __pfx_worker_thread+0x10/0x10
  [    2.523929]  kthread+0x2c6/0x3b0
  [    2.523931]  ? recalc_sigpending+0x15c/0x1e0
  [    2.523934]  ? __pfx_kthread+0x10/0x10
  [    2.523935]  ret_from_fork+0x36e/0x5a0
  [    2.523937]  ? __pfx_ret_from_fork+0x10/0x10
  [    2.523938]  ? __switch_to+0x572/0xdd0
  [    2.523943]  ? __pfx_kthread+0x10/0x10
  [    2.523944]  ret_from_fork_asm+0x1a/0x30
  [    2.523947]  </TASK>
  [    2.523948] 
  [    2.531253] Allocated by task 48:
  [    2.531399]  kasan_save_stack+0x33/0x60
  [    2.531570]  kasan_save_track+0x14/0x30
  [    2.531737]  __kasan_kmalloc+0x8f/0xa0
  [    2.531905]  __kmalloc_cache_noprof+0x158/0x370
  [    2.532100]  smc_llc_enqueue+0x72/0x560
  [    2.532268]  smc_wr_rx_tasklet_fn+0x474/0xa80
  [    2.532491]  tasklet_action_common+0x20f/0x8a0
  [    2.532714]  handle_softirqs+0x18e/0x590
  [    2.532886]  do_softirq+0x3b/0x60
  [    2.533036]  __local_bh_enable_ip+0x61/0x70
  [    2.533221]  __alloc_skb+0x732/0x890
  [    2.533384]  rxe_init_packet+0x16b/0x4f0
  [    2.533567]  prepare_ack_packet+0xb8/0x830
  [    2.533760]  rxe_receiver+0x495/0x96e0
  [    2.533933]  do_work+0x144/0x470
  [    2.534078]  process_one_work+0x633/0x1030
  [    2.534257]  worker_thread+0x45b/0xd10
  [    2.534424]  kthread+0x2c6/0x3b0
  [    2.534569]  ret_from_fork+0x36e/0x5a0
  [    2.534737]  ret_from_fork_asm+0x1a/0x30
  [    2.534907] 
  [    2.534980] Freed by task 11:
  [    2.535112]  kasan_save_stack+0x33/0x60
  [    2.535279]  kasan_save_track+0x14/0x30
  [    2.535444]  kasan_save_free_info+0x3b/0x60
  [    2.535625]  __kasan_slab_free+0x43/0x70
  [    2.535798]  kfree+0x121/0x380
  [    2.535935]  smc_llc_srv_add_link+0x9a8/0x1e50
  [    2.536128]  smc_listen_work+0x489e/0x4d00
  [    2.536305]  process_one_work+0x633/0x1030
  [    2.536482]  worker_thread+0x45b/0xd10
  [    2.536652]  kthread+0x2c6/0x3b0
  [    2.536794]  ret_from_fork+0x36e/0x5a0
  [    2.536958]  ret_from_fork_asm+0x1a/0x30
  [    2.537133] 
  [    2.537205] The buggy address belongs to the object at ffff888005219480
  [    2.537205]  which belongs to the cache kmalloc-96 of size 96
  [    2.537719] The buggy address is located 94 bytes inside of
  [    2.537719]  freed 96-byte region [ffff888005219480, ffff8880052194e0)
  [    2.538216] 
  [    2.538289] The buggy address belongs to the physical page:
  [    2.538524] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5219
  [    2.538857] flags: 0x100000000000000(node=0|zone=1)
  [    2.539066] page_type: f5(slab)
  [    2.539210] raw: 0100000000000000 ffff888001041280 dead000000000122 0000000000000000
  [    2.539534] raw: 0000000000000000 0000000000200020 00000000f5000000 0000000000000000
  [    2.539863] page dumped because: kasan: bad access detected
  [    2.540098] 
  [    2.540170] Memory state around the buggy address:
  [    2.540379]  ffff888005219380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  [    2.540684]  ffff888005219400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  [    2.540988] >ffff888005219480: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
  [    2.541291]                                                     ^
  [    2.541548]  ffff888005219500: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
  [    2.541857]  ffff888005219580: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc

The offset is past the 72-byte queue entry because the out-of-bounds read
fixed by the next patch is on the same line; what this patch removes is the
free at smc_llc_srv_add_link+0x9a8 happening before the read at +0xaa2.

Detach the entry instead of freeing it there, and free it at the single
exit label.  The reject path has to detach as well, otherwise it would be
freed twice.

This changes only the lifetime of the entry.  The same read still runs past
its end until the next two patches bound it, so a backport wants all three.

Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
Changes since v5: return through the existing exit label.

 net/smc/smc_llc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 954b2ff1815c..055a03eee5b5 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -1481,7 +1481,7 @@ int smc_llc_srv_add_link(struct smc_link *link,
 	}
 	add_llc = &qentry->msg.add_link;
 	if (add_llc->hd.flags & SMC_LLC_FLAG_ADD_LNK_REJ) {
-		smc_llc_flow_qentry_del(&lgr->llc_flow_lcl);
+		smc_llc_flow_qentry_clr(&lgr->llc_flow_lcl);
 		rc = -ENOLINK;
 		goto out_err;
 	}
@@ -1492,7 +1492,8 @@ int smc_llc_srv_add_link(struct smc_link *link,
 		lgr_new_t = SMC_LGR_ASYMMETRIC_PEER;
 	}
 	smc_llc_save_add_link_info(link_new, add_llc);
-	smc_llc_flow_qentry_del(&lgr->llc_flow_lcl);
+	/* add_llc still points into qentry, so only detach it here */
+	smc_llc_flow_qentry_clr(&lgr->llc_flow_lcl);
 
 	rc = smc_ib_ready_link(link_new);
 	if (rc)
@@ -1512,14 +1513,14 @@ int smc_llc_srv_add_link(struct smc_link *link,
 	rc = smc_llc_srv_conf_link(link, link_new, lgr_new_t);
 	if (rc)
 		goto out_err;
-	kfree(ini);
-	return 0;
+	goto out;
 out_err:
 	if (link_new) {
 		link_new->state = SMC_LNK_INACTIVE;
 		smcr_link_clear(link_new, false);
 	}
 out:
+	kfree(qentry);
 	kfree(ini);
 	if (send_req_add_link_resp)
 		smc_llc_send_req_add_link_response(req_qentry);
-- 
2.43.0

[PATCH net v6 2/3] net/smc: bound the peer rkey counts in SMC-Rv2 LLC messages

[Yehyeong Lee]

On a link whose device has max_recv_sge == 1 there is no shared v2 receive
buffer, and smc_llc_save_add_link_rkeys() takes the v2 extension from 44
bytes past the start of the queue entry's inline message:

  ext = (struct smc_llc_msg_add_link_v2_ext *)(llc_msg + SMC_WR_TX_SIZE);

The entry is a 72-byte allocation and the extension starts at offset 68, so
ext->num_rkeys at offset 94 is already past it.  This happens on every
SMC-Rv2 link addition, whatever the peer sends:

  [    2.490065] BUG: KASAN: slab-out-of-bounds in smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.490431] Read of size 2 at addr ffff8880056406de by task smctest/106
  [    2.490709] 
  [    2.490792] CPU: 0 UID: 0 PID: 106 Comm: smctest Not tainted 7.2.0-rc5-p1-g77a5d9d9c99f #32 PREEMPT(lazy) 
  [    2.490795] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
  [    2.490798] Call Trace:
  [    2.490803]  <TASK>
  [    2.490805]  dump_stack_lvl+0x53/0x70
  [    2.490810]  print_report+0xd0/0x630
  [    2.490828]  ? __pfx__raw_spin_lock_irqsave+0x10/0x10
  [    2.490832]  ? smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.490834]  kasan_report+0xce/0x100
  [    2.490836]  ? smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.490837]  smc_llc_save_add_link_rkeys+0x333/0x350
  [    2.490839]  ? smcr_buf_map_lgr+0x1bf/0x2b0
  [    2.490844]  smc_llc_cli_add_link+0xca7/0x1e80
  [    2.490848]  ? smc_llc_wait+0x355/0x810
  [    2.490850]  ? __pfx_smc_llc_wait+0x10/0x10
  [    2.490851]  ? __pfx_smc_llc_cli_add_link+0x10/0x10
  [    2.490853]  ? __pfx_autoremove_wake_function+0x10/0x10
  [    2.490863]  __smc_connect+0x3f5c/0x4980
  [    2.490873]  ? __pfx_kernel_connect+0x10/0x10
  [    2.490888]  ? __pfx___smc_connect+0x10/0x10
  [    2.490891]  ? release_sock+0x148/0x1d0
  [    2.490894]  smc_connect+0x42c/0x580
  [    2.490896]  __sys_connect+0xfc/0x130
  [    2.490898]  ? __pfx___sys_connect+0x10/0x10
  [    2.490900]  ? handle_mm_fault+0x1a1/0x430
  [    2.490908]  __x64_sys_connect+0x6d/0xb0
  [    2.490909]  ? fpregs_assert_state_consistent+0x56/0xe0
  [    2.490917]  do_syscall_64+0xf9/0x540
  [    2.490921]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
  [    2.490924] RIP: 0033:0x421bb4
  [    2.490927] Code: ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 80 3d ad 34 09 00 00 74 13 b8 2a 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 4c c3 0f 1f 00 55 48 89 e5 48 83 ec 10 89 55
  [    2.490929] RSP: 002b:00007ffd473b01a8 EFLAGS: 00000202 ORIG_RAX: 000000000000002a
  [    2.490935] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000421bb4
  [    2.490936] RDX: 0000000000000010 RSI: 00007ffd473b01d0 RDI: 0000000000000003
  [    2.490937] RBP: 0000000000003930 R08: 0000000000000004 R09: 0000000000000000
  [    2.490938] R10: 00007ffd473b0f98 R11: 0000000000000202 R12: 0000000000000006
  [    2.490939] R13: 00007ffd473b0f87 R14: 0000000000000003 R15: 00007ffd473b0f90
  [    2.490940]  </TASK>
  [    2.490941] 
  [    2.499545] Allocated by task 44:
  [    2.499693]  kasan_save_stack+0x33/0x60
  [    2.499860]  kasan_save_track+0x14/0x30
  [    2.500026]  __kasan_kmalloc+0x8f/0xa0
  [    2.500190]  __kmalloc_cache_noprof+0x158/0x370
  [    2.500393]  smc_llc_enqueue+0x72/0x560
  [    2.500559]  smc_wr_rx_tasklet_fn+0x474/0xa80
  [    2.500747]  tasklet_action_common+0x20f/0x8a0
  [    2.500945]  handle_softirqs+0x18e/0x590
  [    2.501115]  do_softirq+0x3b/0x60
  [    2.501266]  __local_bh_enable_ip+0x61/0x70
  [    2.501446]  __alloc_skb+0x732/0x890
  [    2.501604]  rxe_init_packet+0x16b/0x4f0
  [    2.501783]  prepare_ack_packet+0xb8/0x830
  [    2.501962]  rxe_receiver+0x495/0x96e0
  [    2.502125]  do_work+0x144/0x470
  [    2.502269]  process_one_work+0x633/0x1030
  [    2.502450]  worker_thread+0x45b/0xd10
  [    2.502617]  kthread+0x2c6/0x3b0
  [    2.502762]  ret_from_fork+0x36e/0x5a0
  [    2.502925]  ret_from_fork_asm+0x1a/0x30
  [    2.503103] 
  [    2.503177] The buggy address belongs to the object at ffff888005640680
  [    2.503177]  which belongs to the cache kmalloc-96 of size 96
  [    2.503692] The buggy address is located 22 bytes to the right of
  [    2.503692]  allocated 72-byte region [ffff888005640680, ffff8880056406c8)
  [    2.504227] 
  [    2.504300] The buggy address belongs to the physical page:
  [    2.504535] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5640
  [    2.504865] flags: 0x100000000000000(node=0|zone=1)
  [    2.505076] page_type: f5(slab)
  [    2.505221] raw: 0100000000000000 ffff888001041280 dead000000000122 0000000000000000
  [    2.505544] raw: 0000000000000000 0000000000200020 00000000f5000000 0000000000000000
  [    2.505867] page dumped because: kasan: bad access detected
  [    2.506102] 
  [    2.506176] Memory state around the buggy address:
  [    2.506380]  ffff888005640580: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
  [    2.506683]  ffff888005640600: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
  [    2.506987] >ffff888005640680: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
  [    2.507291]                                                     ^
  [    2.507548]  ffff888005640700: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
  [    2.507850]  ffff888005640780: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc

Whatever that read finds then bounds the ext->rt[] loop, so a peer that
declares 255 rkeys reads much further.  smc_llc_rmt_delete_rkey() has the
same shape for llcv2->rkey[].

Bound both loops by the buffer they read from, and skip the extension
altogether when there is no shared v2 receive buffer.  The extension
does arrive on the link, but smc_llc_enqueue() copies only
sizeof(union smc_llc_msg) into the queue entry, so what that code read
past the 44 inline bytes was heap and not peer data.

Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
v4 -> v5: corrected the reason given for skipping the extension.  It does
arrive on the link; what is not there is the copy in the queue entry.  No
functional change.

Measured over rxe with KASAN and max_recv_sge forced to 1, five test cells
(plain 1-rkey delete, delete declaring 255, plain ADD_LINK v2, ADD_LINK
declaring 255, and an SMC-Rv1 link group).  Without this patch four of the
five report; with it none do.  With kasan_multi_shot the unpatched kernel
reports 491 times in a single ADD_LINK run, the patched one not at all.

Changes since v5: none.

 net/smc/smc_llc.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 055a03eee5b5..748d65186f68 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -1000,13 +1000,21 @@ static void smc_llc_save_add_link_rkeys(struct smc_link *link,
 					struct smc_link *link_new,
 					u8 *llc_msg)
 {
+	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))
+		return;
 	ext = (struct smc_llc_msg_add_link_v2_ext *)(llc_msg +
 						     SMC_WR_TX_SIZE);
 	max = min_t(u8, ext->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
+	max = min_t(u32, max, (SMC_WR_BUF_V2_SIZE - SMC_WR_TX_SIZE - rt_off) /
+			      sizeof(ext->rt[0]));
 	down_write(&lgr->rmbs_lock);
 	for (i = 0; i < max; i++) {
 		smc_rtoken_set(lgr, link->link_idx, link_new->link_idx,
@@ -1811,17 +1819,25 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
 	link = qentry->link;
 
 	if (lgr->smc_version == SMC_V2) {
+		const u32 rkey_off =
+			offsetof(struct smc_llc_msg_delete_rkey_v2, rkey);
 		struct smc_llc_msg_delete_rkey_v2 *llcv2;
+		u32 buf_len;
 
 		if (smc_link_shared_v2_rxbuf(link)) {
 			memcpy(lgr->wr_rx_buf_v2, llc, sizeof(*llc));
 			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
+			buf_len = SMC_WR_BUF_V2_SIZE;
 		} else {
 			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
+			buf_len = sizeof(qentry->msg);
 		}
 		llcv2->num_inval_rkeys = 0;
 
 		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
+		/* bound by the buffer llcv2 points at */
+		max = min_t(u32, max, (buf_len - rkey_off) /
+				      sizeof(llcv2->rkey[0]));
 		for (i = 0; i < max; i++) {
 			if (smc_rtoken_delete(link, llcv2->rkey[i]))
 				llcv2->num_inval_rkeys++;
-- 
2.43.0

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

[Yehyeong Lee]

smc_llc_rmt_delete_rkey() and smc_llc_save_add_link_rkeys() read the part
of a v2 message that does not fit into the 44-byte union smc_llc_msg, and
both bound themselves by the size of the buffer it landed in, not by what
arrived.  On a link with a shared v2 receive buffer a 44-byte
DELETE_RKEY_V2 declaring 255 rkeys reaches rkey[9..254] in whatever an
earlier message left in lgr->wr_rx_buf_v2, and passes each of them to
smc_rtoken_delete().  One of those 255 matched a registered rtoken and
deleted it.  An ADD_LINK on such a link installs up to 255 rtokens from
the same bytes.

Copy the tail into the queue entry, so its length is the length of the
message that arrived, and declare the rkeys that fit inline as a member of
the union instead of reaching them through a cast.  The same
DELETE_RKEY_V2 now processes the 9 rkeys it carries.  The copy is limited
to the longest tail the two functions can read, so the peer does not pick
the size of the entry.

The bound the previous patch placed on links without a shared v2 receive
buffer is no longer needed.

Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")
Cc: stable@vger.kernel.org
Suggested-by: D. Wythe <alibuda@linux.alibaba.com>
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
Changes since v5: added the Fixes: and Cc: stable tags; asserted that the two
DELETE_RKEY_V2 layouts agree on offsetof(rkey); limited the copied tail to
what the two readers can use; corrected the comment in smc_wr_init_sge().

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.

Five runs per cell with and without the new limit: a 44-byte DELETE_RKEY_V2
declaring 255 rkeys reports 9 processed on a link with and without a shared
v2 receive buffer, an ADD_LINK v2 extension installs the 6 rtokens the peer
sent, and no KASAN report appears.  The only message the limit changes in
that lab is a REQ_ADD_LINK, which copied 16 bytes that have no reader and
now copies none.  On the unpatched kernel the same DELETE_RKEY_V2 reports
254 and 255, and the ADD_LINK installs 255 rtokens per call.

 net/smc/smc_llc.c | 125 ++++++++++++++++++++++++++++++++--------------
 net/smc/smc_wr.c  |   6 +--
 2 files changed, 91 insertions(+), 40 deletions(-)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 748d65186f68..393aa0af18d1 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -157,6 +157,7 @@ struct smc_llc_msg_confirm_rkey {	/* type 0x06 */
 };
 
 #define SMC_LLC_DEL_RKEY_MAX	8
+#define SMC_LLC_DEL_RKEY_V2_INLINE	9
 #define SMC_LLC_FLAG_RKEY_RETRY	0x10
 #define SMC_LLC_FLAG_RKEY_NEG	0x20
 
@@ -177,6 +178,15 @@ struct smc_llc_msg_delete_rkey_v2 {	/* type 0x29 */
 	__be32 rkey[];
 };
 
+/* the leading rkeys of a DELETE_RKEY_V2 fit into union smc_llc_msg */
+struct smc_llc_msg_delete_rkey_v2_inline {	/* type 0x29 */
+	struct smc_llc_hdr hd;
+	u8 num_rkeys;
+	u8 num_inval_rkeys;
+	u8 reserved[2];
+	__be32 rkey[SMC_LLC_DEL_RKEY_V2_INLINE];
+};
+
 union smc_llc_msg {
 	struct smc_llc_msg_confirm_link confirm_link;
 	struct smc_llc_msg_add_link add_link;
@@ -186,6 +196,7 @@ union smc_llc_msg {
 
 	struct smc_llc_msg_confirm_rkey confirm_rkey;
 	struct smc_llc_msg_delete_rkey delete_rkey;
+	struct smc_llc_msg_delete_rkey_v2_inline delete_rkey_v2;
 
 	struct smc_llc_msg_test_link test_link;
 	struct {
@@ -194,15 +205,25 @@ union smc_llc_msg {
 	} raw;
 };
 
+static_assert(SMC_LLC_DEL_RKEY_V2_INLINE ==
+	      (sizeof(union smc_llc_msg) -
+	       offsetof(struct smc_llc_msg_delete_rkey_v2, rkey)) /
+	      sizeof(__be32));
+static_assert(offsetof(struct smc_llc_msg_delete_rkey_v2_inline, rkey) ==
+	      offsetof(struct smc_llc_msg_delete_rkey_v2, rkey));
+
 #define SMC_LLC_FLAG_RESP		0x80
 
 struct smc_llc_qentry {
 	struct list_head list;
 	struct smc_link *link;
+	u16 body_len;
 	union smc_llc_msg msg;
+	u8 body[] __counted_by(body_len);
 };
 
-static void smc_llc_enqueue(struct smc_link *link, union smc_llc_msg *llc);
+static void smc_llc_enqueue(struct smc_link *link, union smc_llc_msg *llc,
+			    u32 byte_len);
 
 struct smc_llc_qentry *smc_llc_flow_qentry_clr(struct smc_llc_flow *flow)
 {
@@ -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);
-	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++) {
@@ -1107,9 +1125,7 @@ int smc_llc_cli_add_link(struct smc_link *link, struct smc_llc_qentry *qentry)
 	if (rc)
 		goto out_clear_lnk;
 	if (lgr->smc_version == SMC_V2) {
-		u8 *llc_msg = smc_link_shared_v2_rxbuf(link) ?
-			(u8 *)lgr->wr_rx_buf_v2 : (u8 *)llc;
-		smc_llc_save_add_link_rkeys(link, lnk_new, llc_msg);
+		smc_llc_save_add_link_rkeys(link, lnk_new, qentry);
 	} else {
 		rc = smc_llc_cli_rkey_exchange(link, lnk_new);
 		if (rc) {
@@ -1510,9 +1526,7 @@ int smc_llc_srv_add_link(struct smc_link *link,
 	if (rc)
 		goto out_err;
 	if (lgr->smc_version == SMC_V2) {
-		u8 *llc_msg = smc_link_shared_v2_rxbuf(link) ?
-			(u8 *)lgr->wr_rx_buf_v2 : (u8 *)add_llc;
-		smc_llc_save_add_link_rkeys(link, link_new, llc_msg);
+		smc_llc_save_add_link_rkeys(link, link_new, qentry);
 	} else {
 		rc = smc_llc_srv_rkey_exchange(link, link_new);
 		if (rc)
@@ -1561,7 +1575,8 @@ void smc_llc_add_link_local(struct smc_link *link)
 	add_llc.hd.common.llc_type = SMC_LLC_ADD_LINK;
 	smc_llc_init_msg_hdr(&add_llc.hd, link->lgr, sizeof(add_llc));
 	/* no dev and port needed */
-	smc_llc_enqueue(link, (union smc_llc_msg *)&add_llc);
+	smc_llc_enqueue(link, (union smc_llc_msg *)&add_llc,
+			sizeof(union smc_llc_msg));
 }
 
 /* worker to process an add link message */
@@ -1597,7 +1612,8 @@ void smc_llc_srv_delete_link_local(struct smc_link *link, u8 del_link_id)
 	del_llc.link_num = del_link_id;
 	del_llc.reason = htonl(SMC_LLC_DEL_LOST_PATH);
 	del_llc.hd.flags |= SMC_LLC_FLAG_DEL_LINK_ORDERLY;
-	smc_llc_enqueue(link, (union smc_llc_msg *)&del_llc);
+	smc_llc_enqueue(link, (union smc_llc_msg *)&del_llc,
+			sizeof(union smc_llc_msg));
 }
 
 static void smc_llc_process_cli_delete_link(struct smc_link_group *lgr)
@@ -1819,27 +1835,28 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
 	link = qentry->link;
 
 	if (lgr->smc_version == SMC_V2) {
-		const u32 rkey_off =
-			offsetof(struct smc_llc_msg_delete_rkey_v2, rkey);
-		struct smc_llc_msg_delete_rkey_v2 *llcv2;
-		u32 buf_len;
-
-		if (smc_link_shared_v2_rxbuf(link)) {
-			memcpy(lgr->wr_rx_buf_v2, llc, sizeof(*llc));
-			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
-			buf_len = SMC_WR_BUF_V2_SIZE;
-		} else {
-			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
-			buf_len = sizeof(qentry->msg);
-		}
+		struct smc_llc_msg_delete_rkey_v2_inline *llcv2;
+
+		/* The leading SMC_LLC_DEL_RKEY_V2_INLINE rkeys are declared in
+		 * the message itself, any further ones were received into
+		 * qentry->body.
+		 */
+		llcv2 = &qentry->msg.delete_rkey_v2;
 		llcv2->num_inval_rkeys = 0;
 
 		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
-		/* bound by the buffer llcv2 points at */
-		max = min_t(u32, max, (buf_len - rkey_off) /
-				      sizeof(llcv2->rkey[0]));
+		max = min_t(u32, max, SMC_LLC_DEL_RKEY_V2_INLINE +
+				      qentry->body_len / sizeof(__be32));
 		for (i = 0; i < max; i++) {
-			if (smc_rtoken_delete(link, llcv2->rkey[i]))
+			__be32 rkey;
+
+			if (i < SMC_LLC_DEL_RKEY_V2_INLINE)
+				rkey = llcv2->rkey[i];
+			else
+				memcpy(&rkey, qentry->body +
+					      (i - SMC_LLC_DEL_RKEY_V2_INLINE) *
+					      sizeof(rkey), sizeof(rkey));
+			if (smc_rtoken_delete(link, rkey))
 				llcv2->num_inval_rkeys++;
 		}
 		memset(&llc->rkey[0], 0, sizeof(llc->rkey));
@@ -2080,18 +2097,52 @@ static void smc_llc_rx_response(struct smc_link *link,
 	wake_up(&link->lgr->llc_msg_waiter);
 }
 
-static void smc_llc_enqueue(struct smc_link *link, union smc_llc_msg *llc)
+/* the longest tail either reader of qentry->body can use */
+static u32 smc_llc_max_body_len(union smc_llc_msg *llc)
+{
+	switch (llc->raw.hdr.common.llc_type) {
+	case SMC_LLC_ADD_LINK:
+		return offsetof(struct smc_llc_msg_add_link_v2_ext, rt) +
+		       SMC_LLC_RKEYS_PER_MSG_V2 *
+		       sizeof(struct smc_llc_msg_add_link_cont_rt);
+	case SMC_LLC_DELETE_RKEY:
+		return (SMC_LLC_RKEYS_PER_MSG_V2 -
+			SMC_LLC_DEL_RKEY_V2_INLINE) * sizeof(__be32);
+	default:
+		return 0;
+	}
+}
+
+static void smc_llc_enqueue(struct smc_link *link, union smc_llc_msg *llc,
+			    u32 byte_len)
 {
 	struct smc_link_group *lgr = link->lgr;
 	struct smc_llc_qentry *qentry;
 	unsigned long flags;
+	u16 body_len = 0;
+
+	/* V2 messages can be longer than the inline union smc_llc_msg. Carry
+	 * the remainder in the qentry itself, so that its lifetime and its
+	 * length match the message the peer actually sent.
+	 */
+	if (lgr->smc_version == SMC_V2 && byte_len > SMC_WR_TX_SIZE)
+		body_len = min_t(u32, byte_len, SMC_WR_BUF_V2_SIZE) -
+			   SMC_WR_TX_SIZE;
+	body_len = min_t(u32, body_len, smc_llc_max_body_len(llc));
 
-	qentry = kmalloc_obj(*qentry, GFP_ATOMIC);
+	qentry = kmalloc_flex(*qentry, body, body_len, GFP_ATOMIC);
 	if (!qentry)
 		return;
+	qentry->body_len = body_len;
 	qentry->link = link;
 	INIT_LIST_HEAD(&qentry->list);
 	memcpy(&qentry->msg, llc, sizeof(union smc_llc_msg));
+	if (body_len) {
+		u8 *src = smc_link_shared_v2_rxbuf(link) ?
+			(u8 *)lgr->wr_rx_buf_v2 : (u8 *)llc;
+
+		memcpy(qentry->body, src + SMC_WR_TX_SIZE, body_len);
+	}
 
 	/* process responses immediately */
 	if ((llc->raw.hdr.flags & SMC_LLC_FLAG_RESP) &&
@@ -2123,7 +2174,7 @@ static void smc_llc_rx_handler(struct ib_wc *wc, void *buf)
 			return; /* invalid message */
 	}
 
-	smc_llc_enqueue(link, llc);
+	smc_llc_enqueue(link, llc, wc->byte_len);
 }
 
 /***************************** worker, utils *********************************/
diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index 59c92b46945c..97ba46893b17 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -602,9 +602,9 @@ static void smc_wr_init_sge(struct smc_link *lnk)
 
 	/* With SMC-Rv2 there can be messages larger than SMC_WR_TX_SIZE.
 	 * Each ib_recv_wr gets 2 sges, the second one is a spillover buffer
-	 * and the same buffer for all sges. When a larger message arrived then
-	 * the content of the first small sge is copied to the beginning of
-	 * the larger spillover buffer, allowing easy data mapping.
+	 * and the same buffer for all sges. The spillover sge starts at
+	 * SMC_WR_TX_SIZE, so the leading bytes of that buffer are never
+	 * written.
 	 */
 	for (i = 0; i < lnk->wr_rx_cnt; i++) {
 		int x = i * lnk->wr_rx_sge_cnt;
-- 
2.43.0

Re: [PATCH net v6 2/3] net/smc: bound the peer rkey counts in SMC-Rv2 LLC messages

[sashiko-bot]

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

New issues:
- [High] The patch prevents an out-of-bounds read by skipping or clamping the parsing of SMC-Rv2 LLC message extensions when `max_recv_sge == 1`, but this severely breaks the SMC-Rv2 protocol.

Pre-existing issues:
- [High] Hardware overwrite and unprotected mutation of shared SMC-Rv2 LLC receive buffer `lgr->wr_rx_buf_v2` causes data corruption for concurrent or subsequent messages.
- [High] The actual hardware payload length (`wc->byte_len`) is discarded, allowing short packets to trigger parsing of uninitialized or stale data in the V2 extension buffer.
- [High] Missing memory barriers in `smc_rtoken_delete()` when clearing rtokens allows data races with `smc_rtoken_add()`.
--

--- Patch [2]: [PATCH net v6 2/3] net/smc: bound the peer rkey counts in SMC-Rv2 LLC messages ---
commit bfe2c3983098dc0afd1d51be57a1a079c74df3f7
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

net/smc: bound the peer rkey counts in SMC-Rv2 LLC messages

This commit addresses an out-of-bounds read in SMC-Rv2 LLC messages when
operating on a link with max_recv_sge == 1. It prevents reading past the
allocated 44-byte inline message by skipping the extension parsing when
there is no shared v2 receive buffer and bounding the read loops.

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> index 7282f8ad7593d..f04e0392ec96c 100644
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c
> @@ -1000,13 +1000,21 @@ static void smc_llc_save_add_link_rkeys(struct smc_link *link,
>  					struct smc_link *link_new,
>  					u8 *llc_msg)
>  {
> +	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))
> +		return;

[Severity: High]
Does skipping the extension completely when max_recv_sge == 1 break the
SMC-Rv2 protocol?

Since smc_llc_enqueue() only copies 44 bytes into qentry->msg, any V2
extension received when there is no shared receive buffer is discarded. By
returning early here, it seems we completely skip saving the peer's rkeys,
which would cause RDMA operations over the new link to fail.

>  	ext = (struct smc_llc_msg_add_link_v2_ext *)(llc_msg +
>  						     SMC_WR_TX_SIZE);

[Severity: High]
This is a pre-existing issue, but does reading this extension from the
shared receive buffer cause data corruption for subsequent messages?

When max_recv_sge == 2, the completion tasklet smc_wr_rx_process_cqes()
processes the CQE, copies only the first 44 bytes into the event queue,
and immediately re-posts the receive WQE via smc_wr_rx_post().

This hands ownership of lgr->wr_rx_buf_v2 back to the hardware before the
workqueue smc_llc_event_work() processes the extension. If a subsequent
message arrives, the hardware could silently overwrite the extension in
lgr->wr_rx_buf_v2 while the worker thread is reading it here.

>  	max = min_t(u8, ext->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);

[Severity: High]
This is a pre-existing issue, but can this parse uninitialized or stale data
if the packet was short?

In smc_llc_rx_handler(), the code only verifies that the 44-byte base header
was received:

    if (wc->byte_len < sizeof(*llc))
        return; /* short message */

The actual hardware payload length (wc->byte_len) is discarded because
smc_llc_enqueue() only copies the 44-byte structure into the event queue.
When the worker thread runs, it unconditionally parses the V2 extension from
lgr->wr_rx_buf_v2. If the peer sent a message that was exactly 44 bytes but
specified a longer length_v2 in the header, wouldn't the hardware leave
lgr->wr_rx_buf_v2 uninitialized or stale?

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

[ ... ]

> @@ -1811,17 +1819,25 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
>  	link = qentry->link;
>  
>  	if (lgr->smc_version == SMC_V2) {
> +		const u32 rkey_off =
> +			offsetof(struct smc_llc_msg_delete_rkey_v2, rkey);
>  		struct smc_llc_msg_delete_rkey_v2 *llcv2;
> +		u32 buf_len;
>  
>  		if (smc_link_shared_v2_rxbuf(link)) {
>  			memcpy(lgr->wr_rx_buf_v2, llc, sizeof(*llc));
>  			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;

[Severity: High]
This is a pre-existing issue, but does mutating the shared hardware receive
buffer cause data corruption?

By using memcpy to write to lgr->wr_rx_buf_v2 without any synchronization,
the code modifies memory that is actively mapped for DMA RX. If the hardware
concurrently writes a new message into this shared buffer, could this corrupt
the incoming LLC messages?

> +			buf_len = SMC_WR_BUF_V2_SIZE;
>  		} else {
>  			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
> +			buf_len = sizeof(qentry->msg);
>  		}
>  		llcv2->num_inval_rkeys = 0;
>  
>  		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
> +		/* bound by the buffer llcv2 points at */
> +		max = min_t(u32, max, (buf_len - rkey_off) /
> +				      sizeof(llcv2->rkey[0]));

[Severity: High]
When max_recv_sge == 1, buf_len is clamped to sizeof(qentry->msg) which is
44 bytes. Does this artificially limit the number of deleted rkeys to 9?

The peer might have sent up to 255 rkeys to delete, but any remaining rkeys
would be silently ignored, causing a state mismatch with the peer.

>  		for (i = 0; i < max; i++) {
>  			if (smc_rtoken_delete(link, llcv2->rkey[i]))

[Severity: High]
This is a pre-existing issue, but does smc_rtoken_delete() lack memory
barriers, causing data races with smc_rtoken_add()?

In smc_rtoken_delete(), the rtoken fields are cleared and then clear_bit()
is called:

    lgr->rtokens[i][j].rkey = 0;
    lgr->rtokens[i][j].dma_addr = 0;
    clear_bit(i, lgr->rtokens_used_mask);

Because clear_bit() does not imply a memory barrier, the CPU can reorder the
bit clear before the zeroing stores.

If another thread concurrently executes smc_rtoken_add() (for example, while
processing a CLC accept/confirm message), it might observe the cleared bit,
reserve the slot, and initialize it:

    i = smc_rmb_reserve_rtoken_idx(lgr);
    lgr->rtokens[i][lnk->link_idx].rkey = rkey;

Could the delayed zeroing stores from smc_rtoken_delete() then silently
overwrite these newly initialized fields?

>  				llcv2->num_inval_rkeys++;

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

Re: [PATCH net v6 1/3] net/smc: fix use-after-free of the LLC qentry in smc_llc_srv_add_link()

[sashiko-bot]

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

Pre-existing issues:
- [High] Out-of-bounds memory read occurs when processing SMC-R V2 ADD_LINK messages on interfaces with max_recv_sge == 1, accessing V2 extension fields beyond the allocated size of the queue entry.
--

--- Patch [1]: [PATCH net v6 1/3] net/smc: fix use-after-free of the LLC qentry in smc_llc_srv_add_link() ---
commit b6d39643c17322840e48f9be07db4e2efaa719d8
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

net/smc: fix use-after-free of the LLC qentry in smc_llc_srv_add_link()

smc_llc_srv_add_link() keeps add_llc pointing into the queue entry:

  add_llc = &qentry->msg.add_link;                      smc_llc.c:1482
  ...
  smc_llc_save_add_link_info(link_new, add_llc);        smc_llc.c:1494
  smc_llc_flow_qentry_del(&lgr->llc_flow_lcl);          smc_llc.c:1495
  ...
  u8 *llc_msg = smc_link_shared_v2_rxbuf(link) ?
        (u8 *)lgr->wr_rx_buf_v2 : (u8 *)add_llc;        smc_llc.c:1504
  smc_llc_save_add_link_rkeys(link, link_new, llc_msg); smc_llc.c:1506

smc_llc_flow_qentry_del() kfree()s the entry, so on a link without a shared
v2 receive buffer the pointer handed to smc_llc_save_add_link_rkeys() is
already freed.  Before the Fixes: commit that branch always used
lgr->wr_rx_buf_v2 and add_llc was not used after the free.

[ ... ]

The offset is past the 72-byte queue entry because the out-of-bounds read
fixed by the next patch is on the same line; what this patch removes is the
free at smc_llc_srv_add_link+0x9a8 happening before the read at +0xaa2.

Detach the entry instead of freeing it there, and free it at the single
exit label.  The reject path has to detach as well, otherwise it would be
freed twice.

This changes only the lifetime of the entry.  The same read still runs past
its end until the next two patches bound it, so a backport wants all three.

Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> index aa6d83af55eda..7282f8ad7593d 100644
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c
[ ... ]
> @@ -1492,7 +1492,8 @@ int smc_llc_srv_add_link(struct smc_link *link,
>  		lgr_new_t = SMC_LGR_ASYMMETRIC_PEER;
>  	}
>  	smc_llc_save_add_link_info(link_new, add_llc);
> -	smc_llc_flow_qentry_del(&lgr->llc_flow_lcl);
> +	/* add_llc still points into qentry, so only detach it here */
> +	smc_llc_flow_qentry_clr(&lgr->llc_flow_lcl);

[Severity: High]
This is a pre-existing issue, but does reading the V2 extension payload from
qentry result in an out-of-bounds read?

As noted in the commit message, this patch fixes a use-after-free on this
struct but explicitly leaves an out-of-bounds read to be fixed in subsequent
patches.

When a peer sends an SMC-R V2 ADD_LINK message with a V2 extension to an
interface where max_recv_sge == 1, smc_llc_srv_add_link() (and its client
equivalent smc_llc_cli_add_link()) does not use a shared V2 receive buffer.
Instead, it computes llc_msg + SMC_WR_TX_SIZE:

net/smc/smc_llc.c:smc_llc_save_add_link_rkeys() {
    ...
    ext = (struct smc_llc_msg_add_link_v2_ext *)(llc_msg +
                                                 SMC_WR_TX_SIZE);
    max = min_t(u8, ext->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
    ...
}

However, smc_llc_enqueue() allocates only enough space for the base structure
(including the 44-byte msg union):

net/smc/smc_llc.c:smc_llc_enqueue() {
    ...
    qentry = kmalloc_obj(*qentry, GFP_ATOMIC);
    ...
    memcpy(&qentry->msg, llc, sizeof(union smc_llc_msg));
    ...
}

This truncates any received V2 extension payload during copying. Since
SMC_WR_TX_SIZE is 44 bytes, the ext pointer is placed out of bounds of the
qentry structure, causing ext->num_rkeys to read uninitialized slab memory or
KASAN redzones.

Since this can be triggered remotely by a network peer over the RoCE fabric,
is there a concern that applying this patch without the subsequent fixes
leaves a remotely reachable out-of-bounds read?

>  
>  	rc = smc_ib_ready_link(link_new);
>  	if (rc)

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Data race due to lockless modifications of rtoken state in the remote LLC flow.
- [High] Hardware-level data corruption due to sharing a single DMA spillover buffer across all receive WQEs.
--

--- Patch [3]: [PATCH net v6 3/3] net/smc: carry oversized SMC-Rv2 LLC messages in the queue entry ---
commit 189076dfcd2735a0769a31ecfff64742ead50fcf
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

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

This commit updates SMC-Rv2 to copy the tail of oversized LLC messages
into the queue entry itself instead of relying on a shared receive buffer
bound by the arrival time. This allows operations like ADD_LINK and
DELETE_RKEY_V2 to process all declared rkeys correctly without being limited
by the buffer size.

Cc: stable@vger.kernel.org

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c

[ ... ]

> @@ -1819,27 +1835,28 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
>  		max = min_t(u32, max, SMC_LLC_DEL_RKEY_V2_INLINE +
>  				      qentry->body_len / sizeof(__be32));
>  		for (i = 0; i < max; i++) {
> -			if (smc_rtoken_delete(link, llcv2->rkey[i]))
> +			__be32 rkey;
> +
> +			if (i < SMC_LLC_DEL_RKEY_V2_INLINE)
> +				rkey = llcv2->rkey[i];
> +			else
> +				memcpy(&rkey, qentry->body +
> +					      (i - SMC_LLC_DEL_RKEY_V2_INLINE) *
> +					      sizeof(rkey), sizeof(rkey));
> +			if (smc_rtoken_delete(link, rkey))

[Severity: High]
This isn't a bug introduced by this patch, but does calling
smc_rtoken_delete() here introduce a data race?

Looking at smc_rtoken_delete() in net/smc/smc_core.c, it zeroes the
lgr->rtokens array and calls clear_bit() on lgr->rtokens_used_mask without
holding the lgr->rmbs_lock.

Concurrently, another thread executing the local flow like
smc_llc_srv_add_link() -> smc_rtoken_set() will acquire
down_write(&lgr->rmbs_lock) to modify these exact same structures.

Could this lack of locking in the remote flow lead to rtoken state
corruption?

>  				llcv2->num_inval_rkeys++;
>  		}

[ ... ]

> @@ -2081,18 +2098,52 @@ static void smc_llc_rx_response(struct smc_link *link,
>  	qentry->link = link;
>  	INIT_LIST_HEAD(&qentry->list);
>  	memcpy(&qentry->msg, llc, sizeof(union smc_llc_msg));
> +	if (body_len) {
> +		u8 *src = smc_link_shared_v2_rxbuf(link) ?
> +			(u8 *)lgr->wr_rx_buf_v2 : (u8 *)llc;
> +
> +		memcpy(qentry->body, src + SMC_WR_TX_SIZE, body_len);
> +	}

[Severity: High]
This is a pre-existing issue, but is it safe to copy from this shared buffer
if multiple oversized messages arrive back-to-back?

In smc_wr_init_sge() (in net/smc/smc_wr.c), every receive WQE has its second
SGE mapped to the exact same shared DMA address (lnk->wr_rx_v2_dma_addr +
SMC_WR_TX_SIZE).

Because the hardware HCA processes incoming packets at wire speed
independently of CPU polling, if a remote peer sends multiple oversized LLC
messages (like back-to-back DELETE_RKEY_V2 messages), won't the hardware DMA
the tails of both messages into this exact same shared spillover buffer?

Could this cause the earlier message's tail to be overwritten before this
memcpy executes, leading to state corruption from reading incorrect body data?

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