[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.