Re: [PATCH net] net/smc: fix socket refcount leak in smc_switch_conns()

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

Pre-existing issues:
- [High] Failover is aborted and the backup link is unnecessarily torn down if a single connection is closed or aborted during the process.
--

--- Patch [1]: [PATCH net] net/smc: fix socket refcount leak in smc_switch_conns() ---
commit 92ea50109006ec40a22d7ea0ca6e43139666208a
Author: Hidayath Khan <[email protected]>

net/smc: fix socket refcount leak in smc_switch_conns()

This commit fixes a socket refcount leak in smc_switch_conns(). It adds a
missing sock_put() call on an early error exit path where
smc_cdc_get_free_slot() fails.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1148,8 +1148,10 @@ struct smc_link *smc_switch_conns(struct smc_link_group *lgr,
>  		read_unlock_bh(&lgr->conns_lock);
>  		/* pre-fetch buffer outside of send_lock, might sleep */
>  		rc = smc_cdc_get_free_slot(conn, to_lnk, &wr_buf, NULL, &pend);
> -		if (rc)
> +		if (rc) {
> +			sock_put(&smc->sk);
>  			goto err_out;
> +		}

[Severity: High]
This is a pre-existing issue, but could this early exit path unintentionally
disrupt all other active connections on the link group?

In smc_switch_conns(), the code iterates through connections to migrate them
to the backup link. If a single connection is aborted concurrently (e.g.,
by remote input causing smc_conn_abort_work to run), smc_cdc_get_free_slot()
detects that the connection was killed and returns -EPIPE.

By jumping directly to err_out, the code calls smcr_link_down_cond_sched()
and tears down the entire backup link. Shouldn't the loop skip the single
dead connection and continue migrating the remaining healthy connections,
rather than aborting the failover for the whole link group?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1