Re: [PATCH net v2 0/2] net/smc: fix diag dump lifetime races

[Mahanta Jambigi <mjambigi@linux.ibm.com>]

On 31/08/26 7:12 pm, Dust Li wrote:
> On 2026-08-28 08:54:37, Mahanta Jambigi wrote:
>> This series fixes multiple lifetime races in the SMC diag dump path.
>>
>> The first patch adds the basic infrastructure needed to synchronize diag readers
>> against connection-owned conn->lgr/conn->lnk updates. It introduces a
>> per-connection spinlock and uses it in the link switch and connection free
>> handoff paths. conn->lgr and conn->lnk are NULLed under the lock before the
>> borrowed references are released, so a non-NULL conn->lgr seen under the lock
>> guarantees the lgr object is alive. The diag reader can rely on this invariant
>> without borrowing any extra reference.
>>
>> The second patch fixes two races in smc_diag itself:
>>
>> - serialize clcsock field access against smc_clcsock_release() with
>>  mutex_trylock()
>> - take conn->lgr_lnk_lock when reading conn->lgr and conn->lnk; use
>>  smc_conn_lgr_valid() inside the lock to check that the connection is
>>  still registered, then snapshot all required fields and call nla_put()
>>  after releasing the lock
> 
> Hi Mahanta,
> 
> As discussed in the other thread, I think we should defer the release of
> smc->clcsock and remove clcsock_release_lock.
> 
> In that case, we should no longer need these two patches. Also,
> introducing more locks in SMC is the last thing I want to do :)

Thanks for the new series "[RFC net-next 0/7] net/smc: tie clcsock
lifetime to the smc socket and remove clcsock_release_lock" — once it
lands, we can drop the mutex_trylock() fix for Race 1 (clcsock).

However, Race 2 remains open. Your series does not touch smc_core.c or
smc_cdc.c, so smc_conn_free(), smc_switch_link_and_count(), and
smc_cdc_msg_validate() still write conn->lgr/conn->lnk with no
synchronization against the diag reader.

On the lock concern — lgr_lnk_lock is a per-connection spinlock. It is
taken in smc_conn_free() (teardown), smc_switch_link_and_count() (link
failover), smc_cdc_msg_validate() (failover validation branch only, not
the normal CDC data path), and the diag reader. None of these are on the
per-message send/receive hot path. Could you explain what specifically
concerns you — lock ordering, memory footprint, or something else? That
would help us understand whether you have a different mechanism in mind.