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

On 04/08/26 2:36 pm, Breno Leitao wrote:

On Tue, Aug 04, 2026 at 10:28:00AM +0200, Hidayath Khan wrote:

quoted

smc_switch_conns() takes a reference on the SMC socket before dropping
lgr->conns_lock, so the connection stays alive while the CDC slot is
fetched:

         sock_hold(&smc->sk);
         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)
                 goto err_out;

The err_out label only drops the wr_tx link reference, so this early exit
returns without the matching sock_put().  The second error exit is not
affected because sock_put() has already run by then:

         rc = smc_switch_cursor(smc, pend, wr_buf);
         spin_unlock_bh(&conn->send_lock);
         sock_put(&smc->sk);
         if (rc)
                 goto err_out;

A leaked sk_refcnt means the smc_sock is never destroyed.  Its send and
receive buffers stay allocated, and for a user socket the reference held
on the network namespace is never released, so the netns can no longer be
torn down.

smc_cdc_get_free_slot() fails when the target link goes down or when the
connection has been killed while the switch is in progress.  Both are
reachable during the link failover this function implements, so the leak
is triggered by the same hardware events that make smc_switch_conns() run
in the first place.

Drop the reference on the early error path.

Fixes: 95f7f3e7dc6b ("net/smc: improved fix wait on already cleared link")
Cc: stable@vger.kernel.org
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
Signed-off-by: Hidayath Khan <redacted>

Reviewed-by: Breno Leitao <leitao@debian.org>

quoted

---
  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 b4208cb186c5..c0027d2fe4e8 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);

Do you need sock_hold(smc->sk) to call smc_cdc_get_free_slot ? Otherwise
you can move the sock_hold() after the exit.

Thanks for the review.

Yes.  conns_lock is what pins the socket. The reference is taken in
smc_lgr_register_conn() and dropped in __smc_lgr_unregister_conn(), both
under that lock.  After read_unlock_bh() a concurrent close can free it,
and smc_cdc_get_free_slot() reads conn->killed after a sleeping
wait_event_interruptible_timeout().  Moving the hold later would turn the
leak into a use-after-free.