[PATCH net] net/smc: guard the CDC receive path against an unset RMB

[PATCH net] net/smc: guard the CDC receive path against an unset RMB

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

The SMC CDC receive handlers look up a connection by its wire token and
then dereference conn->rmb_desc -- smcd_cdc_rx_tsklet() reads
rmb_desc->cpu_addr and smc_cdc_msg_recv_action() reads rmb_desc->len --
but a connection is published to the link group's token tree in
smc_conn_create(), before smc_buf_create() allocates its RMB.  A peer
that sends a CDC message carrying the connection's token in that setup
window reaches the handlers with conn->rmb_desc still NULL and crashes
the host with a NULL pointer dereference.

Bail out of both receive handlers when the RMB is not yet allocated, as
they already do for a killed or out-of-sync connection.

Closes: https://sashiko.dev/#/patchset/20260705-b4-disp-28a1bbca-v4-1-be089b98acc6@proton.me?part=1
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Reachability is by source inspection: smc_conn_create() registers the
connection before smc_buf_create() allocates conn->rmb_desc, and the
receive handlers guard only !conn/out_of_sync/killed.  Verified with an
in-kernel KASAN model reproducing the deref with rmb_desc == NULL (fault
at the ->len / ->cpu_addr offsets) and clean with the guard.  af_smc runs
over an RDMA fabric or an ISM device, so this is the model rather than a
live trigger; reproducer available on request.

No Fixes: tag: the CDC receive path predates the history available here;
the introducing commits are the original CDC support and its SMC-D
addition -- please add whichever tag you prefer.
---
 net/smc/smc_cdc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index 32d6d03df321..d60f68facbc3 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -446,7 +446,7 @@ static void smcd_cdc_rx_tsklet(struct tasklet_struct *t)
 	struct smcd_cdc_msg cdc;
 	struct smc_sock *smc;
 
-	if (!conn || conn->killed)
+	if (!conn || conn->killed || !conn->rmb_desc)
 		return;
 
 	data_cdc = (struct smcd_cdc_msg *)conn->rmb_desc->cpu_addr;
@@ -483,7 +483,7 @@ static void smc_cdc_rx_handler(struct ib_wc *wc, void *buf)
 	lgr = smc_get_lgr(link);
 	read_lock_bh(&lgr->conns_lock);
 	conn = smc_lgr_find_conn(ntohl(cdc->token), lgr);
-	if (!conn || conn->out_of_sync) {
+	if (!conn || conn->out_of_sync || !conn->rmb_desc) {
 		read_unlock_bh(&lgr->conns_lock);
 		return;
 	}

---
base-commit: dd3210c47e8d3ac6b4e9141fc68acc03b38c0ba3
change-id: 20260711-b4-disp-c36a9798-1b35bd69fd72

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>

Re: [PATCH net] net/smc: guard the CDC receive path against an unset RMB

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added check for `!conn->rmb_desc` lacks memory barriers, resulting in a data race. The writer (`__smc_buf_create()`) assigns `conn->rmb_desc = buf_desc` without a write barrier (e.g., `smp_store_release()`) and outside of any lock shared with the reader. Concurrently, the readers (`smc_cdc_rx_handler()` and `smcd_cdc_rx_tsklet()`) perform a plain load of `conn->rmb_desc` without a read barrier (e.g., `smp_load_acquire()` or `READ_ONCE()`). On weakly-ordered architectures, the reader can observe a non-NULL pointer but see uninitialized contents of `buf_desc` (like `cpu_addr` and `len`), causing memory corruption or a NULL pointer dereference.
--

--- Patch [1]: [PATCH net] net/smc: guard the CDC receive path against an unset RMB ---
commit d9db6cbb0ec5bacaa514e8123311e01c799a00da
Author: Bryam Vargas <hexlabsecurity@proton.me>
Subject: net/smc: guard the CDC receive path against an unset RMB

This commit guards against a NULL pointer dereference in SMC CDC receive
handlers when a connection's RMB is not yet allocated. It bails out of
the handlers if conn->rmb_desc is NULL.

Link: https://sashiko.dev/#/patchset/20260705-b4-disp-28a1bbca-v4-1-be089b98acc6@proton.me?part=1

> diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
> index 32d6d03df3214..d60f68facbc34 100644
> --- a/net/smc/smc_cdc.c
> +++ b/net/smc/smc_cdc.c
> @@ -446,7 +446,7 @@ static void smcd_cdc_rx_tsklet(struct tasklet_struct *t)
>  	struct smcd_cdc_msg cdc;
>  	struct smc_sock *smc;
>  
> -	if (!conn || conn->killed)
> +	if (!conn || conn->killed || !conn->rmb_desc)

[Severity: High]
Is there a potential data race with __smc_buf_create() here?

The writer __smc_buf_create() assigns conn->rmb_desc = buf_desc without a
write barrier (e.g., smp_store_release()), and outside of any lock shared
with this reader.

If this plain load observes a non-NULL pointer, could weakly-ordered
architectures see uninitialized contents of buf_desc (like cpu_addr and len)?

Would it be safer to use smp_load_acquire() here and smp_store_release() on
the writer side?

>  		return;
>  
>  	data_cdc = (struct smcd_cdc_msg *)conn->rmb_desc->cpu_addr;

[ ... ]

> @@ -483,7 +483,7 @@ static void smc_cdc_rx_handler(struct ib_wc *wc, void *buf)
>  	lgr = smc_get_lgr(link);
>  	read_lock_bh(&lgr->conns_lock);
>  	conn = smc_lgr_find_conn(ntohl(cdc->token), lgr);
> -	if (!conn || conn->out_of_sync) {
> +	if (!conn || conn->out_of_sync || !conn->rmb_desc) {

[Severity: High]
Does this check need the same read barrier protection?

Even though this is inside read_lock_bh(&lgr->conns_lock), the writer
__smc_buf_create() does not hold this lock when assigning the descriptor.

>  		read_unlock_bh(&lgr->conns_lock);
>  		return;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-b4-disp-c36a9798-v1-1-340b0c6053fb@proton.me?part=1