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

[sashiko-bot@kernel.org]

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.
--

commit ea8d2d358addfe3a2e07acf0170476ee1325ff01
Author: Hidayath Khan <hidayath@linux.ibm.com>

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/

> 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
[ ... ]
> @@ -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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907114721.1303498-1-hidayath@linux.ibm.com?part=1