Re: [PATCH net v2] net/smc: fix abort_work termination in smc_conn_free()

On 08/09/26 5:17 pm, sashiko-bot@kernel.org wrote:

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

Pre-existing issues:
- [Critical] Late-queued `tx_work` can execute on a freed socket because it does not hold a socket reference, causing a use-after-free.
--

Thanks for the Sashiko AI review. The finding is correct.

tx_work is the only one of the three connection work items with no socket
reference:

   close_work   sock_hold smc_cdc.c:420   sock_put smc_close.c:444
   abort_work   sock_hold smc_cdc.c:324   sock_put smc_core.c:1754
   tx_work        none                                  none

The link group holds a reference while the connection is registered, which
is what hides this. smc_conn_free() drops it in smc_lgr_unregister_conn()
and never cancels tx_work - it only cancels abort_work - and the link
group usually outlives the socket, so lgr->tx_wq is still there to run an
item that was queued late.

Worth noting the fix in this patch does not transfer. conn->freed works
for abort_work because abort_work holds a reference, so the socket is
alive and the flag is safe to read. tx_work holds nothing, so by the time
it runs there may be no conn to test. It needs the missing reference
rather than a guard: sock_hold() at the three queue sites, sock_put() in
smc_tx_work(), and the reference returned at the cancel sites that report
they removed a pending item.

I will send that separately, since it is a different work item and a
different fix. This patch is unchanged.

Thanks,
Hidayath

commit ea8d2d358addfe3a2e07acf0170476ee1325ff01
Author: Hidayath Khan [off-list ref]

net/smc: fix abort_work termination in smc_conn_free()

This commit addresses deadlocks, reference leaks, and late-queued work races
related to abort_work in smc_conn_free(). It drops the socket lock around
cancel_work_sync() and uses a freed flag to prevent late-queued work from
processing a destroyed connection.

Link: https://lore.kernel.org/netdev/20260806081549.595001-1-hidayath@linux.ibm.com/ (local)

quoted

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 04aedd957543a..9a109eae73b92 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c

[ ... ]

quoted

@@ -1276,8 +1295,7 @@ void smc_conn_free(struct smc_connection *conn)
  			smcd_buf_detach(conn);
  	} else {
  		smc_cdc_wait_pend_tx_wr(conn);
-		if (current_work() != &conn->abort_work)
-			cancel_work_sync(&conn->abort_work);
+		smc_conn_cancel_abort_work(conn);

[Severity: Critical]
This is a pre-existing issue, but while fixing the exact same design flaw for
abort_work, does this leave tx_work vulnerable to a similar late-queue race?

If a local user closes an SMC socket, smc_close_cancel_work() cancels tx_work.
However, before the socket is fully destroyed, a remote peer sending a CDC
message (or a local user reading spliced data triggering
smc_rx_pipe_buf_release()) can cause the incoming message handler
smc_cdc_msg_recv_action() to call smc_tx_pending(). This can re-queue the work
after it was cancelled:

net/smc/smc_tx.c:smc_tx_consumer_update() {
     ...
     queue_delayed_work(conn->lgr->tx_wq, &conn->tx_work, SMC_TX_WORK_DELAY);
     ...
}

Because tx_work does not hold a socket reference, it can be queued late and
outlive the socket. If the socket destruction completes and frees the socket,
and then tx_work expires and executes, it will dereference the freed socket
memory:

net/smc/smc_tx.c:smc_tx_work() {
     ...
     lock_sock(&smc->sk);
     ...
}

Could this lead to a use-after-free if it is reliably reachable by a local
unprivileged user via splice() or by a remote peer sending timed CDC messages
during socket teardown?

quoted

  	}
  	if (!list_empty(&lgr->list)) {
  		smc_buf_unuse(conn, lgr); /* allow buffer reuse */