Re: [PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path
Mahanta Jambigi <[email protected]>
| Newsgroups | org.kernel.vger.linux-s390,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 07/08/26 1:46 pm, Mahanta Jambigi wrote:
> Two races in the SMC diag dump path:
>
> Race 1: smc_diag_msg_common_fill() reads smc->clcsock fields after a
> NULL check, but smc_clcsock_release() can set clcsock = NULL under
> clcsock_release_lock between the check and the reads. Hold the same
> mutex in the read path to make the check and reads atomic.
>
> Race 2: __smc_diag_dump() dereferences conn->lgr and conn->lnk with no
> protection against concurrent teardown. smc_close_active_abort() calls
> smc_conn_free() — which drops lgr and link refcounts — without first
> unhashing the socket, leaving stale pointers visible to the dump. The
> teardown path holds lock_sock(sk) across smc_conn_free(); take the same
> lock in __smc_diag_dump() to serialise fully.
>
> Both fixes require sleeping locks, which are illegal under the
> read_lock(&smc_hash->lock) held by smc_diag_dump_proto(). Pin each
> socket with refcount_inc_not_zero() before dropping the hash lock, call
> __smc_diag_dump() locklessly, then release the pin. Restart sk_for_each()
> from head after each unlock rather than resuming mid-walk:
> smc_unhash_sk() nulls sk->sk_node.next via sk_del_node_init(), so
> resuming an interrupted walk silently truncates the dump.
>
> Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
> Fixes: 9dbe086c69b8 ("net/smc: fix invalid link access in dumping SMC-R connections")
> Reviewed-by: Sidraya Jayagond <[email protected]>
> Signed-off-by: Mahanta Jambigi <[email protected]>
Hi D. Wythe and team,
I have addressed the review comments from syzbot and Sashiko for this
patch. Could you please review this when convenient?
Also, the following patch is related to the current patch and may be
reviewed together:
https://lore.kernel.org/netdev/[email protected]/