[PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[Yifei Gao]

smc_switch_conns() takes a reference on the socket with sock_hold() before
dropping conns_lock to prefetch a tx slot. On success the reference is
released by the sock_put() after smc_switch_cursor(). When
smc_cdc_get_free_slot() fails, however, the function jumps straight to
err_out without a sock_put(), leaking the reference and leaving the socket
unreclaimable. During SMC-R link failover with an exhausted backup link
this leaks one reference per failing connection switch.

Release the reference before jumping to err_out. The err_out label is also
reached from the smc_switch_cursor() failure path, which has already done
its own sock_put(), so the put must stay at the get_free_slot failure site
rather than at the label.

Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Yifei Gao <gyf161023@gmail.com>
---
 net/smc/smc_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index cf6b620fef05..04c6ce608ec7 100644
--- 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;
+		}
 		/* avoid race with smcr_tx_sndbuf_nonempty() */
 		spin_lock_bh(&conn->send_lock);
 		smc_switch_link_and_count(conn, to_lnk);
-- 
2.43.0

Re: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[Tony Lu]

> Subject: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

Please use [PATCH net] as the subject prefix when sending v2, since
this fix targets the net tree.

On Tue, Aug 04, 2026 at 09:39:17PM +0000, Yifei Gao wrote:
> smc_switch_conns() takes a reference on the socket with sock_hold() before
> dropping conns_lock to prefetch a tx slot. On success the reference is
> released by the sock_put() after smc_switch_cursor(). When
> smc_cdc_get_free_slot() fails, however, the function jumps straight to
> err_out without a sock_put(), leaking the reference and leaving the socket
> unreclaimable. During SMC-R link failover with an exhausted backup link
> this leaks one reference per failing connection switch.
> 
> Release the reference before jumping to err_out. The err_out label is also
> reached from the smc_switch_cursor() failure path, which has already done
> its own sock_put(), so the put must stay at the get_free_slot failure site
> rather than at the label.
> 
> Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Yifei Gao <gyf161023@gmail.com>
> ---
>  net/smc/smc_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index cf6b620fef05..04c6ce608ec7 100644
> --- 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;
> +		}
>  		/* avoid race with smcr_tx_sndbuf_nonempty() */
>  		spin_lock_bh(&conn->send_lock);
>  		smc_switch_link_and_count(conn, to_lnk);
> -- 
> 2.43.0

Re: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[Mahanta Jambigi]

On 05/08/26 3:09 am, Yifei Gao wrote:
> smc_switch_conns() takes a reference on the socket with sock_hold() before
> dropping conns_lock to prefetch a tx slot. On success the reference is
> released by the sock_put() after smc_switch_cursor(). When
> smc_cdc_get_free_slot() fails, however, the function jumps straight to
> err_out without a sock_put(), leaking the reference and leaving the socket
> unreclaimable. During SMC-R link failover with an exhausted backup link
> this leaks one reference per failing connection switch.
> 
> Release the reference before jumping to err_out. The err_out label is also
> reached from the smc_switch_cursor() failure path, which has already done
> its own sock_put(), so the put must stay at the get_free_slot failure site
> rather than at the label.
> 
> Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Yifei Gao <gyf161023@gmail.com>
> ---
>  net/smc/smc_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index cf6b620fef05..04c6ce608ec7 100644
> --- 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;
> +		}

This patch has already been posted by Hidayath:

https://lore.kernel.org/netdev/20260804082800.498672-1-hidayath@linux.ibm.com/

Adding Hidayath to confirm, but this appears to be a duplicate submission.

Re: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[Hidayathulla Khan I]

Hi Yifei,

Thanks for looking into this.

Yes I have submitted a patch for the same issue.
https://lore.kernel.org/netdev/20260804082800.498672-1-hidayath@linux.ibm.com/

Hidayath

On 05/08/26 3:09 am, Yifei Gao wrote:
> smc_switch_conns() takes a reference on the socket with sock_hold() before
> dropping conns_lock to prefetch a tx slot. On success the reference is
> released by the sock_put() after smc_switch_cursor(). When
> smc_cdc_get_free_slot() fails, however, the function jumps straight to
> err_out without a sock_put(), leaking the reference and leaving the socket
> unreclaimable. During SMC-R link failover with an exhausted backup link
> this leaks one reference per failing connection switch.
>
> Release the reference before jumping to err_out. The err_out label is also
> reached from the smc_switch_cursor() failure path, which has already done
> its own sock_put(), so the put must stay at the get_free_slot failure site
> rather than at the label.
>
> Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Yifei Gao <gyf161023@gmail.com>
> ---
>   net/smc/smc_core.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index cf6b620fef05..04c6ce608ec7 100644
> --- 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;
> +		}
>   		/* avoid race with smcr_tx_sndbuf_nonempty() */
>   		spin_lock_bh(&conn->send_lock);
>   		smc_switch_link_and_count(conn, to_lnk);

Re: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[sashiko-bot]

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

Pre-existing issues:
- [High] A concurrent connection abort during failover causes the healthy backup link to be erroneously torn down, failing the entire link group failover.
--

--- Patch [1]: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path ---
commit 1858eaf3cce20ce7bd2bd6af774eda2e7fe87f8d
Author: Yifei Gao <gyf161023@gmail.com>

net/smc: fix sock refcount leak in smc_switch_conns() error path

This commit resolves a socket reference count leak in smc_switch_conns()
by ensuring sock_put() is called when smc_cdc_get_free_slot() fails.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index b4208cb186c5e..c0027d2fe4e8c 100644
> --- 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 isn't a bug introduced by this patch, but this modified error path
appears to have a pre-existing logic issue during a concurrent abort.

If a concurrent abort event occurs (for example, if smc_cdc_msg_validate()
detects an invalid CDC sequence number) and sets conn->killed = 1,
smc_cdc_get_free_slot() will observe this and return -EPIPE.

Since smc_switch_conns() does not distinguish this connection-specific
error from a link-level error, it will jump directly to err_out and call
smcr_link_down_cond_sched(to_lnk).

Does this inadvertently tear down the healthy backup link and disrupt all
other connections in the link group that were successfully failing over,
just because a single connection was killed?

>  		/* avoid race with smcr_tx_sndbuf_nonempty() */
>  		spin_lock_bh(&conn->send_lock);
>  		smc_switch_link_and_count(conn, to_lnk);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804213917.3413525-1-gyf161023@gmail.com?part=1

Re: [PATCH] net/smc: fix sock refcount leak in smc_switch_conns() error path

[高逸飞]

Hi Hidayath,

Thanks for the fix — I see yours covers the same issue and was posted
first. Please disregard my duplicate.


Hidayathulla Khan I <hidayath@linux.ibm.com> 于2026年8月5日周三 02:26写道：
>
> Hi Yifei,
>
> Thanks for looking into this.
>
> Yes I have submitted a patch for the same issue.
> https://lore.kernel.org/netdev/20260804082800.498672-1-hidayath@linux.ibm.com/
>
> Hidayath
>
> On 05/08/26 3:09 am, Yifei Gao wrote:
> > smc_switch_conns() takes a reference on the socket with sock_hold() before
> > dropping conns_lock to prefetch a tx slot. On success the reference is
> > released by the sock_put() after smc_switch_cursor(). When
> > smc_cdc_get_free_slot() fails, however, the function jumps straight to
> > err_out without a sock_put(), leaking the reference and leaving the socket
> > unreclaimable. During SMC-R link failover with an exhausted backup link
> > this leaks one reference per failing connection switch.
> >
> > Release the reference before jumping to err_out. The err_out label is also
> > reached from the smc_switch_cursor() failure path, which has already done
> > its own sock_put(), so the put must stay at the get_free_slot failure site
> > rather than at the label.
> >
> > Fixes: c6f02ebeea3a ("net/smc: switch connections to alternate link")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Yifei Gao <gyf161023@gmail.com>
> > ---
> >   net/smc/smc_core.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> > index cf6b620fef05..04c6ce608ec7 100644
> > --- 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;
> > +             }
> >               /* avoid race with smcr_tx_sndbuf_nonempty() */
> >               spin_lock_bh(&conn->send_lock);
> >               smc_switch_link_and_count(conn, to_lnk);