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

[email protected]
Newsgroups org.kernel.vger.linux-s390
Message-ID <[email protected]>
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 <[email protected]>
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/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.