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