[PATCH net] net/smc: drain the rx tasklet before detaching the ghost sndbuf

Bryam Vargas <[email protected]>
Newsgroups org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.kernel.vger.linux-rdma,org.kernel.vger.linux-s390,org.kernel.vger.netdev
Message-ID <[email protected]>
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: [email protected]
Signed-off-by: Bryam Vargas <[email protected]>
---
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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.