* Re: [syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path
2026-08-07 15:02 ` [syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path syzbot ci
@ 2026-08-11 8:03 ` Mahanta Jambigi
2026-08-11 8:09 ` syzbot ci
2026-08-13 7:25 ` Mahanta Jambigi
1 sibling, 1 reply; 5+ messages in thread
From: Mahanta Jambigi @ 2026-08-11 8:03 UTC (permalink / raw)
To: syzbot ci; +Cc: syzbot, ",syzkaller-bugs"
[-- 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path
2026-08-07 15:02 ` [syzbot ci] Re: net/smc: fix clcsock and lgr/lnk races in smc_diag dump path syzbot ci
2026-08-11 8:03 ` Mahanta Jambigi
@ 2026-08-13 7:25 ` Mahanta Jambigi
1 sibling, 0 replies; 5+ messages in thread
From: Mahanta Jambigi @ 2026-08-13 7:25 UTC (permalink / raw)
To: syzbot ci, alibuda, andrew, davem, dust.li, edumazet, guwen,
horms, kuba, linux-s390, netdev, pabeni, pasic, sidraya, tonylu
Cc: syzbot, syzkaller-bugs
On 07/08/26 8:32 pm, syzbot ci wrote:
> 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
I have fixed this syzbot issue by submitting a separate fix. I also
tested the fix by submitting the patch to syzbot & after that this issue
is not seen anymore. One can see the test details with a fixed patch in
below link. I'll send that patch separately to netdev shortly.
https://ci.syzbot.org/series/ab0d12fd-741a-4a0f-9f49-e52b84e94f4a
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread