Re: [PATCH net v11] net/smc: fix lgr/lnk lifetime vs diag reader race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] smc: UAF on link group due to re-arming free_work after cancellation
--

commit 14f4015f5f30a5352d7329e4e8f9201358d5aebb
Author: Mahanta Jambigi <mjambigi@linux.ibm.com>

net/smc: fix lgr/lnk lifetime vs diag reader race

This commit addresses races between the diag reader and connection/lgr/lnk
teardown paths by skipping unhashed or partially freed state, avoiding NULL
pointer dereferences and UAFs.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 9974149659c2f..6735f185cf48c 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1279,10 +1279,9 @@ void smc_conn_free(struct smc_connection *conn)
>  		if (current_work() != &conn->abort_work)
>  			cancel_work_sync(&conn->abort_work);
>  	}
> -	if (!list_empty(&lgr->list)) {
> +	if (!list_empty(&lgr->list))
>  		smc_buf_unuse(conn, lgr); /* allow buffer reuse */
> -		smc_lgr_unregister_conn(conn);
> -	}
> +	smc_lgr_unregister_conn(conn);
>  
>  	if (!lgr->conns_num)
>  		smc_lgr_schedule_free_work(lgr);

[Severity: High]
Can this unconditionally re-arm the free_work on a terminating link group,
leading to a use-after-free?

If a remote peer sends a DECLINE message with SMC_CLC_DECL_SYNCERR,
smc_listen_work() calls smc_lgr_cleanup_early(lgr). This unlinks the LGR
and calls __smc_lgr_terminate(), which cancels lgr->free_work and sets
lgr->terminating = 1.

Next, smc_listen_work() calls smc_conn_abort(), which calls smc_conn_free().
By moving smc_lgr_unregister_conn() outside the list_empty() check, it will
now be executed for the unlinked LGR. This decrements lgr->conns_num to 0,
which triggers smc_lgr_schedule_free_work().

Because smc_lgr_schedule_free_work() only checks !lgr->freeing (and not
lgr->terminating), it successfully re-arms the workqueue on the terminating
LGR:

net/smc/smc_core.c:smc_lgr_schedule_free_work() {
    if (!lgr->freeing) {
        mod_delayed_work(system_percpu_wq, &lgr->free_work, ...);
    }
}

The LGR is then freed by __smc_lgr_terminate(), leaving the re-armed timer
to eventually execute and dereference the freed lgr pointer.

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