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

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

[Yehyeong Lee]

SMC-Rv2 uses two receive-buffer layouts, selected by
smc_link_shared_v2_rxbuf() (i.e. lnk->wr_rx_sge_cnt > 1):

 - shared (device max_recv_sge >= 2): each receive work request posts a
   small primary SGE plus a spillover SGE into the per-link-group
   lgr->wr_rx_buf_v2 (SMC_WR_BUF_V2_SIZE = 8 KiB); a message larger than
   SMC_WR_TX_SIZE lands with its body in wr_rx_buf_v2.

 - non-shared (device max_recv_sge == 1): each receive work request posts
   a single SGE of wr_rx_buflen == SMC_WR_BUF_V2_SIZE, so the whole
   message lands in the per-WR receive buffer.

smc_llc_rx_handler() hands the received buffer to smc_llc_enqueue(),
which copies only sizeof(union smc_llc_msg) bytes into the fixed-size
qentry->msg and queues the request; the LLC request is then processed
later from the llc_event_work work item, after smc_wr_rx_process_cqes()
has already reposted (and thus recycled) the per-WR receive buffer.

For the shared layout the rkey array survives in wr_rx_buf_v2, and
smc_llc_rmt_delete_rkey() and smc_llc_save_add_link_rkeys() read it from
there. For the non-shared layout nothing preserves the message body: both
functions instead read the rkey array from the truncated qentry copy
((struct smc_llc_msg_delete_rkey_v2 *)llc, and (u8 *)add_llc +
SMC_WR_TX_SIZE), running past the end of the ~72-byte qentry allocation.
The loop bound num_rkeys comes straight off the wire and is only capped at
SMC_LLC_RKEYS_PER_MSG_V2 (255), so a peer that sends a DELETE_RKEY or
ADD_LINK message with an oversized rkey count triggers an out-of-bounds
read of up to ~1 KiB (delete_rkey) or ~4 KiB (add_link) past a kmalloc-96
object.

  BUG: KASAN: slab-out-of-bounds in smc_llc_rmt_delete_rkey+0x6a4/0x780
  Read of size 4 at addr ffff8880059ee748 by task kworker/0:0H/11
  Workqueue: events_highpri smc_llc_event_work
  Call Trace:
   dump_stack_lvl+0x53/0x70
   print_report+0xd0/0x630
   kasan_report+0xce/0x100
   smc_llc_rmt_delete_rkey+0x6a4/0x780
   smc_llc_event_handler+0xa13/0xef0
   smc_llc_event_work+0x189/0x260
   process_one_work+0x633/0x1030
   worker_thread+0x45b/0xd10
   kthread+0x2c6/0x3b0
   ret_from_fork+0x36e/0x5a0
   ret_from_fork_asm+0x1a/0x30
  Allocated by task 43:
   __kmalloc_cache_noprof+0x158/0x370
   smc_llc_enqueue+0x72/0x560
   smc_wr_rx_tasklet_fn+0x474/0xa80
   ...
  The buggy address is located 0 bytes to the right of
   allocated 72-byte region [ffff8880059ee700, ffff8880059ee748)

Preserve the whole received message in wr_rx_buf_v2 for the non-shared
layout as well, by copying it in smc_llc_rx_handler() before the receive
work request is reposted, and read the rkey array from wr_rx_buf_v2 in
both layouts. The reads are then bounded by the SMC_WR_BUF_V2_SIZE
buffer. This also removes a latent use-after-free on the SMC-R server
add-link path, where the non-shared code dereferenced add_llc after its
qentry had been freed by smc_llc_flow_qentry_del().

Triggering the bug requires a RoCE device that advertises
max_recv_sge == 1 (a configuration the Fixes: change added support for)
and a peer emitting an oversized rkey count. Reproduced under KASAN with
soft-RoCE (rdma_rxe) after forcing wr_rx_sge_cnt = 1.

Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
 net/smc/smc_llc.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 954b2ff1815c..1c32bf970098 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -1099,9 +1099,8 @@ 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,
+					    (u8 *)lgr->wr_rx_buf_v2);
 	} else {
 		rc = smc_llc_cli_rkey_exchange(link, lnk_new);
 		if (rc) {
@@ -1501,9 +1500,8 @@ 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,
+					    (u8 *)lgr->wr_rx_buf_v2);
 	} else {
 		rc = smc_llc_srv_rkey_exchange(link, link_new);
 		if (rc)
@@ -1812,12 +1810,15 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
 	if (lgr->smc_version == SMC_V2) {
 		struct smc_llc_msg_delete_rkey_v2 *llcv2;
 
-		if (smc_link_shared_v2_rxbuf(link)) {
+		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;
-		} else {
-			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
-		}
+		/* The full message, including the rkey array, has been placed in
+		 * wr_rx_buf_v2 for both rxbuf layouts (shared: header copy above
+		 * + spillover DMA; non-shared: full copy in smc_llc_rx_handler()).
+		 * Reading it from the fixed-size qentry copy would run past its
+		 * end.
+		 */
+		llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
 		llcv2->num_inval_rkeys = 0;
 
 		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
@@ -2104,6 +2105,16 @@ 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 received message 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 message from wr_rx_buf_v2, so the whole
+		 * message must be preserved there now, mirroring the shared-rxbuf
+		 * layout where the body arrives in wr_rx_buf_v2 via spillover DMA.
+		 */
+		if (!smc_link_shared_v2_rxbuf(link))
+			memcpy(link->lgr->wr_rx_buf_v2, llc,
+			       min_t(u32, wc->byte_len, SMC_WR_BUF_V2_SIZE));
 	}
 
 	smc_llc_enqueue(link, llc);
-- 
2.43.0

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

[Yehyeong Lee]

SMC-Rv2 uses two receive-buffer layouts, selected by
smc_link_shared_v2_rxbuf() (i.e. lnk->wr_rx_sge_cnt > 1):

 - shared (device max_recv_sge >= 2): each receive work request posts a
   small primary SGE plus a spillover SGE that DMAs the message body into
   the per-link-group lgr->wr_rx_buf_v2 (SMC_WR_BUF_V2_SIZE = 8 KiB) at
   offset SMC_WR_TX_SIZE.

 - non-shared (device max_recv_sge == 1): each receive work request posts
   a single SGE of wr_rx_buflen == SMC_WR_BUF_V2_SIZE, so the whole
   message lands in the per-WR receive buffer.

smc_llc_rx_handler() hands the received buffer to smc_llc_enqueue(),
which copies only sizeof(union smc_llc_msg) bytes into the fixed-size
qentry->msg and queues the request; the LLC request is then processed
later from the llc_event_work work item, after smc_wr_rx_process_cqes()
has already reposted (and thus recycled) the per-WR receive buffer.

For the shared layout the rkey array survives in wr_rx_buf_v2, and
smc_llc_rmt_delete_rkey() and smc_llc_save_add_link_rkeys() read it from
there. For the non-shared layout nothing preserves the message body: both
functions instead read the rkey array from the truncated qentry copy
((struct smc_llc_msg_delete_rkey_v2 *)llc, and (u8 *)add_llc +
SMC_WR_TX_SIZE), running past the end of the ~72-byte qentry allocation.
The loop bound num_rkeys comes straight off the wire and is only capped at
SMC_LLC_RKEYS_PER_MSG_V2 (255), so a peer that sends a DELETE_RKEY or
ADD_LINK message with an oversized rkey count triggers an out-of-bounds
read of up to ~1 KiB (delete_rkey) or ~4 KiB (add_link) past a kmalloc-96
object.

  BUG: KASAN: slab-out-of-bounds in smc_llc_rmt_delete_rkey+0x6a4/0x780
  Read of size 4 at addr ffff8880059ee748 by task kworker/0:0H/11
  Workqueue: events_highpri smc_llc_event_work
  Call Trace:
   dump_stack_lvl+0x53/0x70
   print_report+0xd0/0x630
   kasan_report+0xce/0x100
   smc_llc_rmt_delete_rkey+0x6a4/0x780
   smc_llc_event_handler+0xa13/0xef0
   smc_llc_event_work+0x189/0x260
   process_one_work+0x633/0x1030
   worker_thread+0x45b/0xd10
   kthread+0x2c6/0x3b0
   ret_from_fork+0x36e/0x5a0
   ret_from_fork_asm+0x1a/0x30
  Allocated by task 43:
   __kmalloc_cache_noprof+0x158/0x370
   smc_llc_enqueue+0x72/0x560
   smc_wr_rx_tasklet_fn+0x474/0xa80
   ...
  The buggy address is located 0 bytes to the right of
   allocated 72-byte region [ffff8880059ee700, ffff8880059ee748)

Preserve the message body for the non-shared layout as well: in
smc_llc_rx_handler() copy the bytes beyond SMC_WR_TX_SIZE into
wr_rx_buf_v2 + SMC_WR_TX_SIZE before the receive work request is reposted,
exactly mirroring the spillover SGE of the shared layout. The header
(which for a message that fits in SMC_WR_TX_SIZE contains the whole rkey
array) is not copied here; smc_llc_rmt_delete_rkey() now restores it from
the per-qentry copy for both layouts, so a different v2 LLC message that
reaches the shared wr_rx_buf_v2 between reception and processing cannot
clobber it. The rkey array is then read from wr_rx_buf_v2, bounded by its
SMC_WR_BUF_V2_SIZE size.

The body copy is guarded by lgr->smc_version == SMC_V2, since
wr_rx_buf_v2 is only allocated for v2 link groups; keying it off the
link-group version rather than only the wire llc_version prevents a NULL
dereference should a v2-versioned LLC message arrive on a v1 link group.
It also removes a latent use-after-free on the SMC-R server add-link path,
where the non-shared code dereferenced add_llc after its qentry had been
freed by smc_llc_flow_qentry_del().

The rkey count is still only capped at SMC_LLC_RKEYS_PER_MSG_V2; the reads
now stay within the 8 KiB buffer regardless, and validating num_rkeys
against the received message length is left as a separate hardening.

Triggering the bug requires a RoCE device that advertises
max_recv_sge == 1 (a configuration the Fixes: change added support for)
and a peer emitting an oversized rkey count. Reproduced under KASAN with
soft-RoCE (rdma_rxe) after forcing wr_rx_sge_cnt = 1.

Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1")
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
v2: guard the wr_rx_buf_v2 copy with lgr->smc_version == SMC_V2 (v1 keyed
    the copy off the wire llc_version only, so a v2-versioned LLC message
    received on a v1 link group could dereference the NULL wr_rx_buf_v2);
    copy only the message body and restore the header from the per-qentry
    copy in smc_llc_rmt_delete_rkey(), so a later v2 message reaching
    wr_rx_buf_v2 cannot clobber it; note num_rkeys is not validated
    against the message length.
    v1: https://lore.kernel.org/netdev/20260723094027.391199-1-yhlee@isslab.korea.ac.kr
 net/smc/smc_llc.c | 45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 954b2ff1815c..84d8d9024efa 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -1099,9 +1099,8 @@ 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,
+					    (u8 *)lgr->wr_rx_buf_v2);
 	} else {
 		rc = smc_llc_cli_rkey_exchange(link, lnk_new);
 		if (rc) {
@@ -1501,9 +1500,8 @@ 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,
+					    (u8 *)lgr->wr_rx_buf_v2);
 	} else {
 		rc = smc_llc_srv_rkey_exchange(link, link_new);
 		if (rc)
@@ -1812,12 +1810,15 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
 	if (lgr->smc_version == SMC_V2) {
 		struct smc_llc_msg_delete_rkey_v2 *llcv2;
 
-		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;
-		} else {
-			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
-		}
+		/* Restore the header - and, for a message that fits in
+		 * SMC_WR_TX_SIZE, the whole rkey array - from the per-qentry copy
+		 * for both rxbuf layouts, so a v2 message that reached
+		 * wr_rx_buf_v2 in the meantime cannot clobber it.  The body of a
+		 * larger message is already in wr_rx_buf_v2 (shared: spillover
+		 * DMA; non-shared: smc_llc_rx_handler()).
+		 */
+		memcpy(lgr->wr_rx_buf_v2, llc, sizeof(*llc));
+		llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
 		llcv2->num_inval_rkeys = 0;
 
 		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
@@ -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);
 	}
 
 	smc_llc_enqueue(link, llc);
-- 
2.43.0

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

[sashiko-bot]

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

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

[sashiko-bot]

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

New issues:
- [Critical] Remote NULL pointer dereference in `smc_llc_rx_handler` when receiving an SMC-Rv2 packet on an SMC-Rv1 link group.
- [High] Data race and payload corruption due to lockless shared buffer overwrite in tasklet context.
- [High] Processing uninitialized or stale memory for V1 messages on V2 link groups.

Pre-existing issues:
- [High] Incomplete fix for out-of-bounds read translates the vulnerability into stale memory processing.
--

--- Patch [1]: [PATCH net] net/smc: fix out-of-bounds read of rkey array in SMC-Rv2 LLC processing ---
commit aefbf14dc11357a5a9793f782a4910a8949cde03
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 addresses an out-of-bounds read in SMC-Rv2 LLC processing by
copying the full message to wr_rx_buf_v2 for the non-shared layout. The
subsequent processing functions are then updated to read the rkey array
from this buffer, bounding the reads to SMC_WR_BUF_V2_SIZE.

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

[ ... ]

> @@ -1812,12 +1810,15 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
>  	if (lgr->smc_version == SMC_V2) {
>  		struct smc_llc_msg_delete_rkey_v2 *llcv2;
>  
> -		if (smc_link_shared_v2_rxbuf(link)) {
> +		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;
> -		} else {
> -			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
> -		}
> +		/* The full message, including the rkey array, has been placed in
> +		 * wr_rx_buf_v2 for both rxbuf layouts (shared: header copy above
> +		 * + spillover DMA; non-shared: full copy in smc_llc_rx_handler()).
> +		 * Reading it from the fixed-size qentry copy would run past its
> +		 * end.
> +		 */
> +		llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;

[Severity: High]
If a peer sends a V1 message (llc_version == 0) over a V2 link, the memcpy
in smc_llc_rx_handler() is bypassed. However, since the link group version
(lgr->smc_version) is SMC_V2, we reach this point and read from
lgr->wr_rx_buf_v2.

Doesn't this mean we are reading and processing stale memory left over from
a previous V2 message, rather than the V1 message just received?

[Severity: High]
Since smc_llc_rx_handler() unconditionally overwrites lgr->wr_rx_buf_v2
from a tasklet without restoring the header from the queued llc copy for
the non-shared layout, what happens if concurrent LLC messages are received?

If a large message (like DELETE_RKEY_V2) is quickly followed by a short
message (like TEST_LINK), couldn't the short message overwrite the first
few bytes of lgr->wr_rx_buf_v2 before the worker thread runs? This would
cause the worker thread to parse corrupted headers for the first message.

>  		llcv2->num_inval_rkeys = 0;
>  
>  		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);

[Severity: High]
This is a pre-existing issue, but this loop relies on the untrusted
num_rkeys without verifying if the received packet is actually large enough
to contain them.

If a truncated message is received with a large num_rkeys,
smc_llc_rx_handler() only copies up to wc->byte_len bytes into
lgr->wr_rx_buf_v2.

Doesn't this loop then read past the newly copied bytes into stale memory
from previous messages left in the buffer?

[ ... ]

> @@ -2104,6 +2105,16 @@ 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 received message 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 message from wr_rx_buf_v2, so the whole
> +		 * message must be preserved there now, mirroring the shared-rxbuf
> +		 * layout where the body arrives in wr_rx_buf_v2 via spillover DMA.
> +		 */
> +		if (!smc_link_shared_v2_rxbuf(link))
> +			memcpy(link->lgr->wr_rx_buf_v2, llc,
> +			       min_t(u32, wc->byte_len, SMC_WR_BUF_V2_SIZE));

[Severity: Critical]
If a remote peer sends an LLC message with a non-zero llc_version on an
SMC-Rv1 link group, we enter this else block.

For V1 links, smc_link_shared_v2_rxbuf(link) evaluates to false because
lnk->wr_rx_sge_cnt is 1, so we execute this memcpy. However,
lgr->wr_rx_buf_v2 is only allocated for V2 link groups and will be NULL
for V1 links.

Could this result in a NULL pointer dereference in the tasklet context if
an attacker sends such a packet on a V1 link?

>  	}
>  
>  	smc_llc_enqueue(link, llc);

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

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

[Yehyeong Lee]

SMC-Rv2 uses two receive-buffer layouts, selected by
smc_link_shared_v2_rxbuf() (i.e. lnk->wr_rx_sge_cnt > 1):

 - shared (device max_recv_sge >= 2): each receive work request posts a
   small primary SGE plus a spillover SGE that DMAs the message body into
   the per-link-group lgr->wr_rx_buf_v2 (SMC_WR_BUF_V2_SIZE = 8 KiB) at
   offset SMC_WR_TX_SIZE.

 - non-shared (device max_recv_sge == 1): each receive work request posts
   a single SGE of wr_rx_buflen == SMC_WR_BUF_V2_SIZE, so the whole
   message lands in the per-WR receive buffer.

smc_llc_rx_handler() hands the received buffer to smc_llc_enqueue(),
which copies only sizeof(union smc_llc_msg) bytes into the fixed-size
qentry->msg and queues the request; the LLC request is then processed
later from the llc_event_work work item, after smc_wr_rx_process_cqes()
has already reposted (and thus recycled) the per-WR receive buffer.

For the shared layout the rkey array survives in wr_rx_buf_v2, and
smc_llc_rmt_delete_rkey() and smc_llc_save_add_link_rkeys() read it from
there. For the non-shared layout nothing preserves the message body: both
functions instead read the rkey array from the truncated qentry copy
((struct smc_llc_msg_delete_rkey_v2 *)llc, and (u8 *)add_llc +
SMC_WR_TX_SIZE), running past the end of the ~72-byte qentry allocation.
The loop bound num_rkeys comes straight off the wire and is only capped at
SMC_LLC_RKEYS_PER_MSG_V2 (255), so a peer that sends a DELETE_RKEY or
ADD_LINK message with an oversized rkey count triggers an out-of-bounds
read of up to ~1 KiB (delete_rkey) or ~4 KiB (add_link) past a kmalloc-96
object.

  BUG: KASAN: slab-out-of-bounds in smc_llc_rmt_delete_rkey+0x6a4/0x780
  Read of size 4 at addr ffff8880059ee748 by task kworker/0:0H/11
  Workqueue: events_highpri smc_llc_event_work
  Call Trace:
   dump_stack_lvl+0x53/0x70
   print_report+0xd0/0x630
   kasan_report+0xce/0x100
   smc_llc_rmt_delete_rkey+0x6a4/0x780
   smc_llc_event_handler+0xa13/0xef0
   smc_llc_event_work+0x189/0x260
   process_one_work+0x633/0x1030
   worker_thread+0x45b/0xd10
   kthread+0x2c6/0x3b0
   ret_from_fork+0x36e/0x5a0
   ret_from_fork_asm+0x1a/0x30
  Allocated by task 43:
   __kmalloc_cache_noprof+0x158/0x370
   smc_llc_enqueue+0x72/0x560
   smc_wr_rx_tasklet_fn+0x474/0xa80
   ...
  The buggy address is located 0 bytes to the right of
   allocated 72-byte region [ffff8880059ee700, ffff8880059ee748)

Preserve the message body for the non-shared layout as well: in
smc_llc_rx_handler() copy the bytes beyond SMC_WR_TX_SIZE into
wr_rx_buf_v2 + SMC_WR_TX_SIZE before the receive work request is reposted,
exactly mirroring the spillover SGE of the shared layout. The header
(which for a message that fits in SMC_WR_TX_SIZE contains the whole rkey
array) is not copied here; smc_llc_rmt_delete_rkey() now restores it from
the per-qentry copy for both layouts, so a different v2 LLC message that
reaches the shared wr_rx_buf_v2 between reception and processing cannot
clobber it. The rkey array is then read from wr_rx_buf_v2, bounded by its
SMC_WR_BUF_V2_SIZE size.

The body copy is guarded by lgr->smc_version == SMC_V2, since
wr_rx_buf_v2 is only allocated for v2 link groups; keying it off the
link-group version rather than only the wire llc_version prevents a NULL
dereference should a v2-versioned LLC message arrive on a v1 link group.
It also removes a latent use-after-free on the SMC-R server add-link path,
where the non-shared code dereferenced add_llc after its qentry had been
freed by smc_llc_flow_qentry_del().

The rkey count is still only capped at SMC_LLC_RKEYS_PER_MSG_V2; the reads
now stay within the 8 KiB buffer regardless, and validating num_rkeys
against the received message length is left as a separate hardening.

Triggering the bug requires a RoCE device that advertises
max_recv_sge == 1 (a configuration the Fixes: change added support for)
and a peer emitting an oversized rkey count. Reproduced under KASAN with
soft-RoCE (rdma_rxe) after forcing wr_rx_sge_cnt = 1.

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

Resent as a standalone thread per pv-bot: threaded.
Previous posting: 20260724045310.430202-1-yhlee@isslab.korea.ac.kr

 net/smc/smc_llc.c | 45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 954b2ff1815c..84d8d9024efa 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -1099,9 +1099,8 @@ 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,
+					    (u8 *)lgr->wr_rx_buf_v2);
 	} else {
 		rc = smc_llc_cli_rkey_exchange(link, lnk_new);
 		if (rc) {
@@ -1501,9 +1500,8 @@ 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,
+					    (u8 *)lgr->wr_rx_buf_v2);
 	} else {
 		rc = smc_llc_srv_rkey_exchange(link, link_new);
 		if (rc)
@@ -1812,12 +1810,15 @@ static void smc_llc_rmt_delete_rkey(struct smc_link_group *lgr)
 	if (lgr->smc_version == SMC_V2) {
 		struct smc_llc_msg_delete_rkey_v2 *llcv2;
 
-		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;
-		} else {
-			llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)llc;
-		}
+		/* Restore the header - and, for a message that fits in
+		 * SMC_WR_TX_SIZE, the whole rkey array - from the per-qentry copy
+		 * for both rxbuf layouts, so a v2 message that reached
+		 * wr_rx_buf_v2 in the meantime cannot clobber it.  The body of a
+		 * larger message is already in wr_rx_buf_v2 (shared: spillover
+		 * DMA; non-shared: smc_llc_rx_handler()).
+		 */
+		memcpy(lgr->wr_rx_buf_v2, llc, sizeof(*llc));
+		llcv2 = (struct smc_llc_msg_delete_rkey_v2 *)lgr->wr_rx_buf_v2;
 		llcv2->num_inval_rkeys = 0;
 
 		max = min_t(u8, llcv2->num_rkeys, SMC_LLC_RKEYS_PER_MSG_V2);
@@ -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);
 	}
 
 	smc_llc_enqueue(link, llc);
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Data Race and Use-After-Overwrite in Asynchronous LLC Message Processing
--

--- Patch [1]: [PATCH net v2] net/smc: fix out-of-bounds read of rkey array in SMC-Rv2 LLC processing ---
commit 63d9ad267ae12085da6001381c4f5b5b34c2bf73
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 when processing SMC-Rv2 LLC messages
with oversized rkey counts in the non-shared receive-buffer layout. It
addresses the issue by copying the message body into the shared wr_rx_buf_v2
in the receive handler before the per-WR buffer is reposted.

> 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: Medium]
If a peer sends two oversized LLC messages in rapid succession, could this
memcpy() execute in the softirq handler while the first message is still
being processed by the smc_llc_event_work workqueue?

Since the workqueue reads from the same shared lgr->wr_rx_buf_v2 buffer:

smc_llc_rmt_delete_rkey() {
    ...
    if (smc_rtoken_delete(link, llcv2->rkey[i]))
    ...
}

Does this code introduce a data race regression causing torn reads, potentially
leading to the processing of mixed or overwritten payloads?

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