[PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[Mahanta Jambigi]

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 <sidraya@linux.ibm.com>
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
 net/smc/smc_diag.c | 78 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index bf0beaa23bdb..000000000000 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -39,22 +39,34 @@
 	memset(r, 0, sizeof(*r));
 	r->diag_family = sk->sk_family;
 	sock_diag_save_cookie(sk, r->id.idiag_cookie);
-	if (!smc->clcsock)
-		return;
-	r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
-	r->id.idiag_dport = smc->clcsock->sk->sk_dport;
-	r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
-	if (sk->sk_protocol == SMCPROTO_SMC) {
-		r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
-		r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
+	/*
+	 * smc_clcsock_release() sets smc->clcsock = NULL under
+	 * clcsock_release_lock before freeing the socket. Hold the same
+	 * mutex here to make the NULL check and all field reads atomic
+	 * with that writer. mutex_lock() is safe: this function is called
+	 * only after the hash spinlock has been dropped by
+	 * smc_diag_dump_proto().
+	 */
+	mutex_lock(&smc->clcsock_release_lock);
+	if (smc->clcsock) {
+		r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
+		r->id.idiag_dport = smc->clcsock->sk->sk_dport;
+		r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
+		if (sk->sk_protocol == SMCPROTO_SMC) {
+			r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
+			r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
 #if IS_ENABLED(CONFIG_IPV6)
-	} else if (sk->sk_protocol == SMCPROTO_SMC6) {
-		memcpy(&r->id.idiag_src, &smc->clcsock->sk->sk_v6_rcv_saddr,
-		       sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
-		memcpy(&r->id.idiag_dst, &smc->clcsock->sk->sk_v6_daddr,
-		       sizeof(smc->clcsock->sk->sk_v6_daddr));
+		} else if (sk->sk_protocol == SMCPROTO_SMC6) {
+			memcpy(&r->id.idiag_src,
+			       &smc->clcsock->sk->sk_v6_rcv_saddr,
+			       sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
+			memcpy(&r->id.idiag_dst,
+			       &smc->clcsock->sk->sk_v6_daddr,
+			       sizeof(smc->clcsock->sk->sk_v6_daddr));
 #endif
+		}
 	}
+	mutex_unlock(&smc->clcsock_release_lock);
 }
 
 static int smc_diag_msg_attrs_fill(struct sock *sk, struct sk_buff *skb,
@@ -87,6 +99,15 @@
 
 	r = nlmsg_data(nlh);
 	smc_diag_msg_common_fill(r, sk);
+	/*
+	 * Take the socket lock to serialise against smc_conn_free(),
+	 * which drops lgr and link refcounts under lock_sock(). Without
+	 * this, a concurrent close can free lgr->lnk[] memory between
+	 * our smc_conn_lgr_valid() check and the subsequent lgr/lnk
+	 * dereferences. lock_sock() is safe here because the hash
+	 * spinlock has been dropped by smc_diag_dump_proto().
+	 */
+	lock_sock(sk);
 	r->diag_state = sk->sk_state;
 	if (smc->use_fallback)
 		r->diag_mode = SMC_DIAG_MODE_FALLBACK_TCP;
@@ -182,13 +203,16 @@
 		dinfo.peer_token = conn->peer_token;
 
 		if (nla_put(skb, SMC_DIAG_DMBINFO, sizeof(dinfo), &dinfo) < 0)
 			goto errout;
 	}
 
+	release_sock(sk);
+
 	nlmsg_end(skb, nlh);
 	return 0;
 
 errout:
+	release_sock(sk);
 	nlmsg_cancel(skb, nlh);
 	return -EMSGSIZE;
 }
@@ -204,25 +228,43 @@
 	int rc = 0, num = 0;
 	struct sock *sk;
 
-	read_lock(&prot->h.smc_hash->lock);
 	head = &prot->h.smc_hash->ht;
+restart:
+	num = 0;
+	read_lock(&prot->h.smc_hash->lock);
 	if (hlist_empty(head))
 		goto out;
-
 	sk_for_each(sk, head) {
 		if (!net_eq(sock_net(sk), net))
 			continue;
 		if (num < snum)
 			goto next;
+		/*
+		 * Pin sk before dropping the lock. refcount_inc_not_zero()
+		 * skips sockets already past their last reference.
+		 * smc_unhash_sk() nulls sk->sk_node.next via sk_del_node_init()
+		 * so resuming an interrupted sk_for_each() would terminate
+		 * early if a socket is unhashed while the lock is dropped.
+		 * Restart from head after each unlock, using snum to skip
+		 * already-dumped entries.
+		 */
+		if (!refcount_inc_not_zero(&sk->sk_refcnt))
+			goto next;
+		read_unlock(&prot->h.smc_hash->lock);
+
 		rc = __smc_diag_dump(sk, skb, cb, nlmsg_data(cb->nlh), bc);
+		sock_put(sk);
+
 		if (rc < 0)
-			goto out;
+			goto out_nolock;
+		snum = num + 1;
+		goto restart;
 next:
 		num++;
 	}
-
 out:
 	read_unlock(&prot->h.smc_hash->lock);
+out_nolock:
 	cb_ctx->pos[p_type] = num;
 	return rc;
 }
-- 
2.50.1

Re: [PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[Mahanta Jambigi]

Apologies for the confusion.

This is a v1 patch. The v3 tag in the subject is a mistake. Please
consider this submission as v1.

[syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[syzbot ci]

syzbot ci has tested the following series

[v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path
https://lore.kernel.org/all/20260807081606.3200128-1-mjambigi@linux.ibm.com
* [PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

and found the following issue:
WARNING in smc_diag_dump_proto

Full report is available here:
https://ci.syzbot.org/series/ab0d12fd-741a-4a0f-9f49-e52b84e94f4a

***

WARNING in smc_diag_dump_proto

tree:      net
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
base:      594d905195024b228c962627ae5ae7c17bd582a4
arch:      amd64
compiler:  Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/0c4d246f-d098-4afe-b6bc-0f0ad52f55f8/config
syz repro: https://ci.syzbot.org/findings/fa5564b8-8db6-4f76-9d75-dbd796435d87/syz_repro

------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: kernel/locking/mutex.c:625 at __mutex_lock_common kernel/locking/mutex.c:625 [inline], CPU#0: syz.0.161/6326
WARNING: kernel/locking/mutex.c:625 at __mutex_lock+0x12d8/0x1550 kernel/locking/mutex.c:821, CPU#0: syz.0.161/6326
Modules linked in:
CPU: 0 UID: 0 PID: 6326 Comm: syz.0.161 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:__mutex_lock_common kernel/locking/mutex.c:625 [inline]
RIP: 0010:__mutex_lock+0x12df/0x1550 kernel/locking/mutex.c:821
Code: de 57 90 48 c1 e8 03 0f b6 04 18 84 c0 0f 85 47 02 00 00 83 3d 45 cc 83 04 00 75 13 48 8d 3d a8 1b 87 04 48 c7 c6 40 f7 ec 8b <67> 48 0f b9 3a 90 e9 75 ee ff ff 90 0f 0b 90 e9 48 f2 ff ff 90 0f
RSP: 0018:ffffc900068c6e40 EFLAGS: 00010246
RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffff888170ffd940
RDX: 0000000000000000 RSI: ffffffff8becf740 RDI: ffffffff905b2db0
RBP: ffffc900068c6fd0 R08: ffffffff9057de43 R09: 1ffffffff20afbc8
R10: dffffc0000000000 R11: fffffbfff20afbc9 R12: ffff8881b1006648
R13: 0000000000000000 R14: 1ffff92000d18ddc R15: 0000000000000000
FS:  00007f4dacd826c0(0000) GS:ffff88818d945000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000020000000e000 CR3: 0000000171350000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 smc_diag_msg_common_fill net/smc/smc_diag.c:50 [inline]
 __smc_diag_dump net/smc/smc_diag.c:101 [inline]
 smc_diag_dump_proto+0x6d2/0x2620 net/smc/smc_diag.c:255
 smc_diag_dump+0x59/0xa0 net/smc/smc_diag.c:278
 netlink_dump+0x711/0xee0 net/netlink/af_netlink.c:2331
 __netlink_dump_start+0x589/0x7b0 net/netlink/af_netlink.c:2446
 netlink_dump_start include/linux/netlink.h:341 [inline]
 smc_diag_handler_dump+0x199/0x240 net/smc/smc_diag.c:293
 sock_diag_rcv_msg+0x4b0/0x5f0 net/core/sock_diag.c:-1
 netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2556
 netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
 netlink_unicast+0x7bb/0x940 net/netlink/af_netlink.c:1345
 netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
 sock_sendmsg_nosec+0x13a/0x180 net/socket.c:775
 __sock_sendmsg net/socket.c:790 [inline]
 ____sys_sendmsg+0x54e/0x850 net/socket.c:2684
 ___sys_sendmsg+0x2a5/0x360 net/socket.c:2738
 __sys_sendmsg net/socket.c:2770 [inline]
 __do_sys_sendmsg net/socket.c:2775 [inline]
 __se_sys_sendmsg net/socket.c:2773 [inline]
 __x64_sys_sendmsg+0x1b1/0x290 net/socket.c:2773
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f4dabf9e019
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f4dacd82028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f4dac226090 RCX: 00007f4dabf9e019
RDX: 0000000000000600 RSI: 0000200000000540 RDI: 0000000000000003
RBP: 00007f4dac03500c R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f4dac226128 R14: 00007f4dac226090 R15: 00007ffe0de8ec98
 </TASK>
----------------
Code disassembly (best guess):
   0:	de 57 90             	ficoms -0x70(%rdi)
   3:	48 c1 e8 03          	shr    $0x3,%rax
   7:	0f b6 04 18          	movzbl (%rax,%rbx,1),%eax
   b:	84 c0                	test   %al,%al
   d:	0f 85 47 02 00 00    	jne    0x25a
  13:	83 3d 45 cc 83 04 00 	cmpl   $0x0,0x483cc45(%rip)        # 0x483cc5f
  1a:	75 13                	jne    0x2f
  1c:	48 8d 3d a8 1b 87 04 	lea    0x4871ba8(%rip),%rdi        # 0x4871bcb
  23:	48 c7 c6 40 f7 ec 8b 	mov    $0xffffffff8becf740,%rsi
* 2a:	67 48 0f b9 3a       	ud1    (%edx),%rdi <-- trapping instruction
  2f:	90                   	nop
  30:	e9 75 ee ff ff       	jmp    0xffffeeaa
  35:	90                   	nop
  36:	0f 0b                	ud2
  38:	90                   	nop
  39:	e9 48 f2 ff ff       	jmp    0xfffff286
  3e:	90                   	nop
  3f:	0f                   	.byte 0xf


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).

The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.

Re: [PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[sashiko-bot]

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

New issues:
- [High] Dropping the hash lock per-socket and unconditionally restarting the list walk from the head using a logical index introduces O(N^2) list traversal complexity and causes active sockets to be skipped in the diag dump.

Pre-existing issues:
- [Critical] The patch attempts to fix a Use-After-Free race on `smc->clcsock` by adding `clcsock_release_lock` in `smc_diag_msg_common_fill()`, but misses a concurrent teardown path in `smc_accept_dequeue()` that frees the socket without acquiring the mutex.
--

--- Patch [1]: [PATCH net v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path ---
commit a16d6de2c1851166c340508a037e5292d66a0907
Author: Mahanta Jambigi <mjambigi@linux.ibm.com>

net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

This commit attempts to fix two races in the SMC diag dump path by holding
clcsock_release_lock in smc_diag_msg_common_fill() and lock_sock() in
__smc_diag_dump(). It also modifies the dump iteration to pin sockets, drop
the hash lock, and restart the traversal from the head of the list.

> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
> index bf0beaa23bdb6..72d77360e504f 100644
> --- a/net/smc/smc_diag.c
> +++ b/net/smc/smc_diag.c
> @@ -39,22 +39,34 @@ static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
>  	memset(r, 0, sizeof(*r));
>  	r->diag_family = sk->sk_family;
>  	sock_diag_save_cookie(sk, r->id.idiag_cookie);
> -	if (!smc->clcsock)
> -		return;
> -	r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
> -	r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
> +	/*
> +	 * smc_clcsock_release() sets smc->clcsock = NULL under
> +	 * clcsock_release_lock before freeing the socket. Hold the same
> +	 * mutex here to make the NULL check and all field reads atomic
> +	 * with that writer. mutex_lock() is safe: this function is called
> +	 * only after the hash spinlock has been dropped by
> +	 * smc_diag_dump_proto().
> +	 */
> +	mutex_lock(&smc->clcsock_release_lock);
> +	if (smc->clcsock) {
> +		r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);

[Severity: Critical]
This is a pre-existing issue, but does this mutex fully protect against
concurrent teardowns?

Looking at smc_accept_dequeue() in net/smc/af_smc.c, if an unaccepted socket
is closed (SMC_CLOSED), it unhashes it and frees isk->clcsock without holding
clcsock_release_lock or the child's socket lock:

net/smc/af_smc.c:smc_accept_dequeue() {
    ...
    if (new_sk->sk_state == SMC_CLOSED) {
        new_sk->sk_prot->unhash(new_sk);
        if (isk->clcsock) {
            sock_release(isk->clcsock);
            isk->clcsock = NULL;
        }
    ...
}

If the netlink diag dump thread concurrently iterates the hash table, grabs
the socket reference, and calls smc_diag_msg_common_fill(), could it acquire
the mutex, see a non-NULL clcsock, and dereference it while it is being freed
by smc_accept_dequeue()?

[ ... ]

> @@ -204,25 +228,43 @@ static int smc_diag_dump_proto(struct proto *prot, struct sk_buff *skb,
>  	int rc = 0, num = 0;
>  	struct sock *sk;
>  
> -	read_lock(&prot->h.smc_hash->lock);
>  	head = &prot->h.smc_hash->ht;
> +restart:
> +	num = 0;
> +	read_lock(&prot->h.smc_hash->lock);
>  	if (hlist_empty(head))
>  		goto out;
> -
>  	sk_for_each(sk, head) {
>  		if (!net_eq(sock_net(sk), net))
>  			continue;
>  		if (num < snum)
>  			goto next;
> +		/*
> +		 * Pin sk before dropping the lock. refcount_inc_not_zero()
> +		 * skips sockets already past their last reference.
> +		 * smc_unhash_sk() nulls sk->sk_node.next via sk_del_node_init()
> +		 * so resuming an interrupted sk_for_each() would terminate
> +		 * early if a socket is unhashed while the lock is dropped.
> +		 * Restart from head after each unlock, using snum to skip
> +		 * already-dumped entries.
> +		 */
> +		if (!refcount_inc_not_zero(&sk->sk_refcnt))
> +			goto next;
> +		read_unlock(&prot->h.smc_hash->lock);
> +
>  		rc = __smc_diag_dump(sk, skb, cb, nlmsg_data(cb->nlh), bc);
> +		sock_put(sk);
> +
>  		if (rc < 0)
> -			goto out;
> +			goto out_nolock;
> +		snum = num + 1;
> +		goto restart;

[Severity: High]
Does restarting the list walk from the head using a logical index (snum)
introduce an O(N^2) list traversal complexity?

If any previously seen sockets are concurrently unhashed while the lock is
dropped, could stepping over (num < snum) elements skip active sockets that
shifted to earlier positions in the list?

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

Re: [syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[Mahanta Jambigi]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

#syz test

v4: fixes two races in the smc_diag dump path (clcsock NULL race and
lgr/lnk teardown race) plus moves mutex_init(&clcsock_release_lock)
before sk->sk_prot->hash(sk) in smc_sk_init() to fix the uninitialized
mutex warning you reported.

[-- Attachment #2: v4-0001-net-smc-fix-clcsock-lgr-lnk-races-and-mutex-init-ordering.patch --]
[-- Type: text/plain, Size: 8436 bytes --]

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Mahanta Jambigi <mjambigi@linux.ibm.com>
Date: Mon, 11 Aug 2026 00:00:00 +0000
Subject: [PATCH net v4] net/smc: fix clcsock and lgr/lnk races in smc_diag
 dump path

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.

Additionally, smc_sk_init() called sk->sk_prot->hash(sk) before
mutex_init(&clcsock_release_lock), creating a window where a concurrent
diag dump could call mutex_lock() on an uninitialized mutex and trigger:

    DEBUG_LOCKS_WARN_ON(lock->magic != lock)
    WARNING: kernel/locking/mutex.c:625

Fix by moving mutex_init() before hash() so the mutex is fully
initialized before the socket becomes visible to concurrent readers.

Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
Fixes: 9dbe086c69b8 ("net/smc: fix invalid link access in dumping SMC-R connections")
Reported-by: syzbot+cia7ddf4f17dfab266@syzkaller.appspotmail.com
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
Changes in v4:
- Move mutex_init(&clcsock_release_lock) before sk->sk_prot->hash(sk)
  in smc_sk_init() to close a window where a concurrent diag dump could
  call mutex_lock() on an uninitialized mutex (syzbot).

Changes in v3:
- Restart sk_for_each() from head after each unlock rather than
  resuming mid-walk. sk_del_node_init() nulls sk->sk_node.next so
  continuing an interrupted hlist walk silently truncates the dump
  (Hidayath Khan).
- Move sock_put() to before read_lock() so a last-reference drop
  cannot run socket teardown under the rwlock (Hidayath Khan).
- Move r->diag_state = sk->sk_state after lock_sock() so diag_state
  and the lgr/lnk fields form a consistent snapshot (Sidraya Jayagond).

Changes in v2:
- Replaces all three patches with a single patch and a simpler
  approach, based on Hidayath's comments.
- Drop read_lock(&smc_hash->lock) before calling __smc_diag_dump()
  and re-acquire it afterwards, using refcount_inc_not_zero() to pin
  the socket across the unlock window (same pattern as tcp_diag).
- With no spinlock held, take lock_sock(sk) in __smc_diag_dump()
  before any lgr or lnk dereference.
- With the spinlock dropped, smc_diag_msg_common_fill() can use
  mutex_lock(&clcsock_release_lock) instead of mutex_trylock().

 net/smc/af_smc.c   |  2 +-
 net/smc/smc_diag.c | 76 +++++++++++++++++++++++++++++++++++-----------

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index b5db69073e20..38a4c009363c 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -409,8 +409,8 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
 				      "sk_lock-AF_SMC", &smc_key);
 	spin_lock_init(&smc->accept_q_lock);
 	spin_lock_init(&smc->conn.send_lock);
-	sk->sk_prot->hash(sk);
 	mutex_init(&smc->clcsock_release_lock);
+	sk->sk_prot->hash(sk);
 	smc_init_saved_callbacks(smc);
 	smc->limit_smc_hs = net->smc.limit_smc_hs;
 	smc->use_fallback = false; /* assume rdma capability first */
diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index bf0beaa23bdb..f3bad0cb09b1 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -39,22 +39,34 @@ static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
 	memset(r, 0, sizeof(*r));
 	r->diag_family = sk->sk_family;
 	sock_diag_save_cookie(sk, r->id.idiag_cookie);
-	if (!smc->clcsock)
-		return;
-	r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
-	r->id.idiag_dport = smc->clcsock->sk->sk_dport;
-	r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
-	if (sk->sk_protocol == SMCPROTO_SMC) {
-		r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
-		r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
+	/*
+	 * smc_clcsock_release() sets smc->clcsock = NULL under
+	 * clcsock_release_lock before freeing the socket. Hold the same
+	 * mutex here to make the NULL check and all field reads atomic
+	 * with that writer. mutex_lock() is safe: this function is called
+	 * only after the hash spinlock has been dropped by
+	 * smc_diag_dump_proto().
+	 */
+	mutex_lock(&smc->clcsock_release_lock);
+	if (smc->clcsock) {
+		r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
+		r->id.idiag_dport = smc->clcsock->sk->sk_dport;
+		r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
+		if (sk->sk_protocol == SMCPROTO_SMC) {
+			r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
+			r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
 #if IS_ENABLED(CONFIG_IPV6)
-	} else if (sk->sk_protocol == SMCPROTO_SMC6) {
-		memcpy(&r->id.idiag_src, &smc->clcsock->sk->sk_v6_rcv_saddr,
-		       sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
-		memcpy(&r->id.idiag_dst, &smc->clcsock->sk->sk_v6_daddr,
-		       sizeof(smc->clcsock->sk->sk_v6_daddr));
+		} else if (sk->sk_protocol == SMCPROTO_SMC6) {
+			memcpy(&r->id.idiag_src,
+			       &smc->clcsock->sk->sk_v6_rcv_saddr,
+			       sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
+			memcpy(&r->id.idiag_dst,
+			       &smc->clcsock->sk->sk_v6_daddr,
+			       sizeof(smc->clcsock->sk->sk_v6_daddr));
 #endif
+		}
 	}
+	mutex_unlock(&smc->clcsock_release_lock);
 }
 
 static int smc_diag_msg_attrs_fill(struct sock *sk, struct sk_buff *skb,
@@ -87,6 +99,15 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
 
 	r = nlmsg_data(nlh);
 	smc_diag_msg_common_fill(r, sk);
+	/*
+	 * Take the socket lock to serialise against smc_conn_free(),
+	 * which drops lgr and link refcounts under lock_sock(). Without
+	 * this, a concurrent close can free lgr->lnk[] memory between
+	 * our smc_conn_lgr_valid() check and the subsequent lgr/lnk
+	 * dereferences. lock_sock() is safe here because the hash
+	 * spinlock has been dropped by smc_diag_dump_proto().
+	 */
+	lock_sock(sk);
 	r->diag_state = sk->sk_state;
 	if (smc->use_fallback)
 		r->diag_mode = SMC_DIAG_MODE_FALLBACK_TCP;
@@ -184,11 +205,12 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
 		if (nla_put(skb, SMC_DIAG_DMBINFO, sizeof(dinfo), &dinfo) < 0)
 			goto errout;
 	}
-
+	release_sock(sk);
 	nlmsg_end(skb, nlh);
 	return 0;
 
 errout:
+	release_sock(sk);
 	nlmsg_cancel(skb, nlh);
 	return -EMSGSIZE;
 }
@@ -204,25 +226,43 @@ static int smc_diag_dump_proto(struct proto *prot, struct sk_buff *skb,
 	int rc = 0, num = 0;
 	struct sock *sk;
 
-	read_lock(&prot->h.smc_hash->lock);
 	head = &prot->h.smc_hash->ht;
+restart:
+	num = 0;
+	read_lock(&prot->h.smc_hash->lock);
 	if (hlist_empty(head))
 		goto out;
-
 	sk_for_each(sk, head) {
 		if (!net_eq(sock_net(sk), net))
 			continue;
 		if (num < snum)
 			goto next;
+		/*
+		 * Pin sk before dropping the lock. refcount_inc_not_zero()
+		 * skips sockets already past their last reference.
+		 * smc_unhash_sk() nulls sk->sk_node.next via sk_del_node_init()
+		 * so resuming an interrupted sk_for_each() would terminate
+		 * early if a socket is unhashed while the lock is dropped.
+		 * Restart from head after each unlock, using snum to skip
+		 * already-dumped entries.
+		 */
+		if (!refcount_inc_not_zero(&sk->sk_refcnt))
+			goto next;
+		read_unlock(&prot->h.smc_hash->lock);
+
 		rc = __smc_diag_dump(sk, skb, cb, nlmsg_data(cb->nlh), bc);
+		sock_put(sk);
+
 		if (rc < 0)
-			goto out;
+			goto out_nolock;
+		snum = num + 1;
+		goto restart;
 next:
 		num++;
 	}
-
 out:
 	read_unlock(&prot->h.smc_hash->lock);
+out_nolock:
 	cb_ctx->pos[p_type] = num;
 	return rc;
 }
-- 
2.50.1

[syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path

[syzbot ci]

syzbot ci has tested the suggested fix patch on top of the following series:

[v3] net/smc: fix clcsock and lgr/lnk races in smc_diag dump path
https://lore.kernel.org/all/20260807081606.3200128-1-mjambigi@linux.ibm.com

Patch: https://ci.syzbot.org/jobs/a680be27-d0df-49ac-a0d3-268f97fd6e34/patch

The patch testing request could not be completed:
Testing failed due to an infrastructure error.
Testing results:
* [build 0] Build Patched: error

Full report is available here:
https://ci.syzbot.org/session/d28d17fc-2477-4003-8c8a-304bd5ad77c9

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.