Re: [PATCH net v3] 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:
- [High] net/smc: zombie abort_work terminates valid fallback connection
--

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

net/smc: fix abort_work termination in smc_conn_free()

This commit switches smc_conn_free() to use a non-blocking cancel_work()
for abort_work to avoid deadlocks. It also adds a check for conn->freed
inside smc_conn_abort_work() and initializes abort_work once per socket
in smc_sk_init() instead of per 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 9974149659c2f..907530e1d6464 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c

[ ... ]

> @@ -1276,8 +1280,13 @@ 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);
> +		/* Do not wait here: the work takes the socket lock this
> +		 * caller holds. An instance that is already running is
> +		 * parked on that lock and will find conn->freed set; only a
> +		 * still-pending one has to give its reference back.
> +		 */
> +		if (cancel_work(&conn->abort_work))
> +			sock_put(&smc->sk);

[Severity: High]
Does changing this to a non-blocking cancel_work() introduce a race with
server-side fallback connections?

During server listen, smc_listen_find_device() iterates over proposed
devices while holding the socket lock. If an initial connection attempt
calls smc_conn_create() but fails and is aborted via smc_conn_free(), an
abort_work queued concurrently could transition to running but block on the
socket lock.

Since cancel_work() does not wait, it would return false for the running
worker and leave it parked on the lock. When the fallback loop continues,
it calls smc_conn_create() for the next device on the same socket, which
unconditionally resets the flag:

net/smc/smc_core.c:smc_conn_create() {
    ...
    conn->freed = 0;
    ...
}

When the fallback connection ultimately succeeds and releases the socket
lock, could the parked abort_work unblock, observe the reset conn->freed
flag, and erroneously call smc_conn_kill() on the new, valid connection?

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

[ ... ]

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