* [PATCH net] net/smc: drain the rx tasklet before detaching the ghost sndbuf
@ 2026-07-27 2:14 Bryam Vargas via B4 Relay
2026-07-29 9:46 ` Dust Li
0 siblings, 1 reply; 2+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-07-27 2:14 UTC (permalink / raw)
To: Dust Li, Wenjia Zhang, D. Wythe, Sidraya Jayagond
Cc: Mahanta Jambigi, netdev, linux-s390, linux-kernel, Tony Lu,
Paolo Abeni, Wen Gu, linux-rdma
From: Bryam Vargas <hexlabsecurity@proton.me>
On the SMC-D DMB-nocopy path smcd_buf_detach() frees conn->sndbuf_desc
while the receive tasklet can still be running: both teardown sites free
first and drain afterwards. A tasklet already past the conn->killed check
then dereferences the freed buffer in smc_cdc_msg_recv_action(), which
reaches the same field again through smc_tx_prepared_sends().
Drain the tasklet before the detach at both sites, mirroring rmb_desc,
which smc_buf_unuse() only releases after the drain.
Fixes: ae2be35cbed2 ("net/smc: {at|de}tach sndbuf to peer DMB if supported")
Cc: stable@kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Found with KASAN on an SMC-D loopback rig (CONFIG_SMC_LO, x86_64).
The teardown is driven through smc_conn_kill() by arming abort_work from a debug
module parameter: on SMC-D nothing in the receive path queues that work, while on
SMC-R smc_cdc_msg_validate() queues it for an out-of-range peer seqno. Everything
after the abort is the unmodified path. A second parameter holds the receive tasklet
just past the conn->killed check, so the free lands mid-run.
arm ordering abort armed result
vulnerable detach then drain yes KASAN slab-use-after-free
control detach then drain no clean
patched drain then detach yes clean
Same module, same boot, same trigger, same delay; only the ordering differs.
BUG: KASAN: slab-use-after-free in smc_cdc_msg_recv_action+0x1ed9/0x1f20 [smc]
Read of size 4 at addr ffff88810f6c9720 by task kworker/3:1/147
Workqueue: smc_close_wq smc_conn_abort_work [smc]
<IRQ>
smc_cdc_msg_recv_action+0x1ed9/0x1f20 [smc]
smcd_cdc_rx_tsklet+0x1f2/0x2f0 [smc]
tasklet_action_common+0x2fd/0x850
handle_softirqs+0x18c/0x4f0
do_softirq+0x3a/0x60
</IRQ>
<TASK>
__local_bh_enable_ip+0x5d/0x60
smc_cdc_get_slot_and_msg_send+0x253/0x680 [smc]
smc_conn_kill+0xa8/0x4d0 [smc]
smc_conn_abort_work+0x34/0x90 [smc]
</TASK>
Allocated by task 70:
smcd_buf_attach+0xbd/0x2a0 [smc]
smc_listen_work+0x2499/0x49a0 [smc]
Freed by task 44:
kfree+0x135/0x390
smc_conn_kill+0x3ab/0x4d0 [smc]
smc_conn_abort_work+0x34/0x90 [smc]
The buggy address belongs to the object at ffff88810f6c9700
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 32 bytes inside of
freed 128-byte region [ffff88810f6c9700, ffff88810f6c9780)
The read is conn->sndbuf_desc->len.
A plain close() does not get there: smc_close_active() always moves the socket to
SMC_PEERCLOSEWAIT1, so the free happens in smc_close_passive_work() after the peer's
close CDC -- the same event that stops feeding the tasklet. About 18000 such teardowns
produced nothing. The ordering is wrong either way, and smc_conn_kill() has no such
coupling.
Logs on request.
---
net/smc/smc_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index cf6b620fef05..da64e7a6bd73 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1270,9 +1270,9 @@ void smc_conn_free(struct smc_connection *conn)
if (lgr->is_smcd) {
if (!list_empty(&lgr->list))
smc_ism_unset_conn(conn);
+ tasklet_kill(&conn->rx_tsklet);
if (smc_ism_support_dmb_nocopy(lgr->smcd))
smcd_buf_detach(conn);
- tasklet_kill(&conn->rx_tsklet);
} else {
smc_cdc_wait_pend_tx_wr(conn);
if (current_work() != &conn->abort_work)
@@ -1525,12 +1525,12 @@ static void smc_conn_kill(struct smc_connection *conn, bool soft)
smc_sk_wake_ups(smc);
if (conn->lgr->is_smcd) {
smc_ism_unset_conn(conn);
- if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
- smcd_buf_detach(conn);
if (soft)
tasklet_kill(&conn->rx_tsklet);
else
tasklet_unlock_wait(&conn->rx_tsklet);
+ if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
+ smcd_buf_detach(conn);
} else {
smc_cdc_wait_pend_tx_wr(conn);
}
---
base-commit: 4235cb24ec1e8e96843f3671ba4da2a6ccca2c7b
change-id: 20260726-b4-disp-a135b4e5-945574bd0938
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net] net/smc: drain the rx tasklet before detaching the ghost sndbuf
2026-07-27 2:14 [PATCH net] net/smc: drain the rx tasklet before detaching the ghost sndbuf Bryam Vargas via B4 Relay
@ 2026-07-29 9:46 ` Dust Li
0 siblings, 0 replies; 2+ messages in thread
From: Dust Li @ 2026-07-29 9:46 UTC (permalink / raw)
To: hexlabsecurity, Wenjia Zhang, D. Wythe, Sidraya Jayagond
Cc: Mahanta Jambigi, netdev, linux-s390, linux-kernel, Tony Lu,
Paolo Abeni, Wen Gu, linux-rdma
On 2026-07-26 21:14:04, Bryam Vargas via B4 Relay wrote:
>From: Bryam Vargas <hexlabsecurity@proton.me>
>
>On the SMC-D DMB-nocopy path smcd_buf_detach() frees conn->sndbuf_desc
>while the receive tasklet can still be running: both teardown sites free
>first and drain afterwards. A tasklet already past the conn->killed check
>then dereferences the freed buffer in smc_cdc_msg_recv_action(), which
>reaches the same field again through smc_tx_prepared_sends().
>
>Drain the tasklet before the detach at both sites, mirroring rmb_desc,
>which smc_buf_unuse() only releases after the drain.
>
>Fixes: ae2be35cbed2 ("net/smc: {at|de}tach sndbuf to peer DMB if supported")
>Cc: stable@kernel.org
>Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
>---
>Found with KASAN on an SMC-D loopback rig (CONFIG_SMC_LO, x86_64).
>
>The teardown is driven through smc_conn_kill() by arming abort_work from a debug
>module parameter: on SMC-D nothing in the receive path queues that work, while on
>SMC-R smc_cdc_msg_validate() queues it for an out-of-range peer seqno. Everything
>after the abort is the unmodified path. A second parameter holds the receive tasklet
>just past the conn->killed check, so the free lands mid-run.
>
> arm ordering abort armed result
> vulnerable detach then drain yes KASAN slab-use-after-free
> control detach then drain no clean
> patched drain then detach yes clean
>
>Same module, same boot, same trigger, same delay; only the ordering differs.
>
> BUG: KASAN: slab-use-after-free in smc_cdc_msg_recv_action+0x1ed9/0x1f20 [smc]
> Read of size 4 at addr ffff88810f6c9720 by task kworker/3:1/147
> Workqueue: smc_close_wq smc_conn_abort_work [smc]
> <IRQ>
> smc_cdc_msg_recv_action+0x1ed9/0x1f20 [smc]
> smcd_cdc_rx_tsklet+0x1f2/0x2f0 [smc]
> tasklet_action_common+0x2fd/0x850
> handle_softirqs+0x18c/0x4f0
> do_softirq+0x3a/0x60
> </IRQ>
> <TASK>
> __local_bh_enable_ip+0x5d/0x60
> smc_cdc_get_slot_and_msg_send+0x253/0x680 [smc]
> smc_conn_kill+0xa8/0x4d0 [smc]
> smc_conn_abort_work+0x34/0x90 [smc]
> </TASK>
>
> Allocated by task 70:
> smcd_buf_attach+0xbd/0x2a0 [smc]
> smc_listen_work+0x2499/0x49a0 [smc]
>
> Freed by task 44:
> kfree+0x135/0x390
> smc_conn_kill+0x3ab/0x4d0 [smc]
> smc_conn_abort_work+0x34/0x90 [smc]
>
> The buggy address belongs to the object at ffff88810f6c9700
> which belongs to the cache kmalloc-128 of size 128
> The buggy address is located 32 bytes inside of
> freed 128-byte region [ffff88810f6c9700, ffff88810f6c9780)
>
>The read is conn->sndbuf_desc->len.
>
>A plain close() does not get there: smc_close_active() always moves the socket to
>SMC_PEERCLOSEWAIT1, so the free happens in smc_close_passive_work() after the peer's
>close CDC -- the same event that stops feeding the tasklet. About 18000 such teardowns
>produced nothing. The ordering is wrong either way, and smc_conn_kill() has no such
>coupling.
>
>Logs on request.
>---
> net/smc/smc_core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
>index cf6b620fef05..da64e7a6bd73 100644
>--- a/net/smc/smc_core.c
>+++ b/net/smc/smc_core.c
>@@ -1270,9 +1270,9 @@ void smc_conn_free(struct smc_connection *conn)
> if (lgr->is_smcd) {
> if (!list_empty(&lgr->list))
> smc_ism_unset_conn(conn);
>+ tasklet_kill(&conn->rx_tsklet);
> if (smc_ism_support_dmb_nocopy(lgr->smcd))
> smcd_buf_detach(conn);
>- tasklet_kill(&conn->rx_tsklet);
> } else {
> smc_cdc_wait_pend_tx_wr(conn);
> if (current_work() != &conn->abort_work)
>@@ -1525,12 +1525,12 @@ static void smc_conn_kill(struct smc_connection *conn, bool soft)
> smc_sk_wake_ups(smc);
> if (conn->lgr->is_smcd) {
> smc_ism_unset_conn(conn);
>- if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
>- smcd_buf_detach(conn);
> if (soft)
> tasklet_kill(&conn->rx_tsklet);
> else
> tasklet_unlock_wait(&conn->rx_tsklet);
>+ if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
>+ smcd_buf_detach(conn);
Hi Bryam,
I think Sashiko's review makes sense. Your change doesn't seem to fully
eliminate the race here. I'm wondering if this should be solved with
RCU rather than just moving smcd_buf_detach() later?
Best regards,
Dust
> } else {
> smc_cdc_wait_pend_tx_wr(conn);
> }
>
>---
>base-commit: 4235cb24ec1e8e96843f3671ba4da2a6ccca2c7b
>change-id: 20260726-b4-disp-a135b4e5-945574bd0938
>
>Best regards,
>--
>Bryam Vargas <hexlabsecurity@proton.me>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-29 9:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 2:14 [PATCH net] net/smc: drain the rx tasklet before detaching the ghost sndbuf Bryam Vargas via B4 Relay
2026-07-29 9:46 ` Dust Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox