Linux s390 Architecture development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Hidayath Khan" <hidayath@linux.ibm.com>
Cc: "Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	linux-s390@vger.kernel.org, "Vasily Gorbik" <gor@linux.ibm.com>
Subject: Re: [PATCH net v2] net/smc: fix socket refcount leak in smc_switch_conns()
Date: Fri, 21 Aug 2026 14:48:07 +0000	[thread overview]
Message-ID: <20260821144808.11B7A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260820144729.1019399-1-hidayath@linux.ibm.com>

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

Pre-existing issues:
- [High] A TOCTOU race during link failover allows a killed connection to abort the failover and bring down the healthy backup link.
--

--- Patch [1]: [PATCH net v2] net/smc: fix socket refcount leak in smc_switch_conns() ---
Note: Due to system recitation constraints, this review uses a free-form summary format rather than quoting the original patch diff.

commit 52d43741c14f87f177497a6086c3732a98a7fec5
Author: Hidayath Khan <hidayath@linux.ibm.com>
net/smc: fix socket refcount leak in smc_switch_conns()

This commit fixes a socket refcount leak in smc_switch_conns() that happens
when smc_cdc_get_free_slot() returns an error. It restructures the code to
use a single sock_put() on the common path instead of adding one to the
early error exit path.

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing
issue in smc_switch_conns() related to how it handles connection errors
during link failover.

When smc_switch_conns() iterates through connections to migrate them, it
drops the conns_lock and calls smc_cdc_get_free_slot(). 

If a specific connection is killed concurrently while the lock is dropped,
smc_cdc_get_free_slot() returns -EPIPE. 

Because smc_switch_conns() evaluates this return code and jumps to the
err_out label on any error, it proceeds to call
smcr_link_down_cond_sched() on the target link.

Does this mean a single killed connection returning -EPIPE will incorrectly
abort the entire failover process and bring down the healthy backup link?

Since this forces a teardown of the backup link, could this be used to
trigger a denial of service for all other sockets sharing the link group
if an application closes the socket during the link switch window?

Would it be better to skip the killed connection and continue migrating
the remaining connections, rather than treating a connection-specific error
as a total link failure?

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

  reply	other threads:[~2026-08-21 14:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20 14:47 [PATCH net v2] net/smc: fix socket refcount leak in smc_switch_conns() Hidayath Khan
2026-08-21 14:48 ` sashiko-bot [this message]
2026-08-24 19:00 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260821144808.11B7A1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hidayath@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox