netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 net] tipc: Fix use-after-free in tipc_mon_reinit_self().
@ 2025-11-06  5:32 Kuniyuki Iwashima
  2025-11-06  9:38 ` [syzbot ci] " syzbot ci
  0 siblings, 1 reply; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-11-06  5:32 UTC (permalink / raw)
  To: Jon Maloy, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Simon Horman, Hoang Le, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev, tipc-discussion, syzbot+d7dad7fd4b3921104957

syzbot reported use-after-free of tipc_net(net)->monitors[]
in tipc_mon_reinit_self().

The array is protected by RTNL, but tipc_mon_reinit_self()
iterates over it without RTNL.

Let's hold RTNL in tipc_mon_reinit_self().

[0]:
BUG: KASAN: slab-use-after-free in __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave+0xa7/0xf0 kernel/locking/spinlock.c:162
Read of size 1 at addr ffff88805eae1030 by task kworker/0:7/5989
CPU: 0 UID: 0 PID: 5989 Comm: kworker/0:7 Not tainted syzkaller #0 PREEMPT_{RT,(full)}
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
Workqueue: events tipc_net_finalize_work
Call Trace:
 <TASK>
 dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0xca/0x240 mm/kasan/report.c:482
 kasan_report+0x118/0x150 mm/kasan/report.c:595
 __kasan_check_byte+0x2a/0x40 mm/kasan/common.c:568
 kasan_check_byte include/linux/kasan.h:399 [inline]
 lock_acquire+0x8d/0x360 kernel/locking/lockdep.c:5842
 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
 _raw_spin_lock_irqsave+0xa7/0xf0 kernel/locking/spinlock.c:162
 rtlock_slowlock kernel/locking/rtmutex.c:1894 [inline]
 rwbase_rtmutex_lock_state kernel/locking/spinlock_rt.c:160 [inline]
 rwbase_write_lock+0xd3/0x7e0 kernel/locking/rwbase_rt.c:244
 rt_write_lock+0x76/0x110 kernel/locking/spinlock_rt.c:243
 write_lock_bh include/linux/rwlock_rt.h:99 [inline]
 tipc_mon_reinit_self+0x79/0x430 net/tipc/monitor.c:718
 tipc_net_finalize+0x115/0x190 net/tipc/net.c:140
 process_one_work kernel/workqueue.c:3236 [inline]
 process_scheduled_works+0xade/0x17b0 kernel/workqueue.c:3319
 worker_thread+0x8a0/0xda0 kernel/workqueue.c:3400
 kthread+0x70e/0x8a0 kernel/kthread.c:463
 ret_from_fork+0x439/0x7d0 arch/x86/kernel/process.c:148
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>

Allocated by task 6089:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3e/0x80 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:388 [inline]
 __kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:405
 kasan_kmalloc include/linux/kasan.h:260 [inline]
 __kmalloc_cache_noprof+0x1a8/0x320 mm/slub.c:4407
 kmalloc_noprof include/linux/slab.h:905 [inline]
 kzalloc_noprof include/linux/slab.h:1039 [inline]
 tipc_mon_create+0xc3/0x4d0 net/tipc/monitor.c:657
 tipc_enable_bearer net/tipc/bearer.c:357 [inline]
 __tipc_nl_bearer_enable+0xe16/0x13f0 net/tipc/bearer.c:1047
 __tipc_nl_compat_doit net/tipc/netlink_compat.c:371 [inline]
 tipc_nl_compat_doit+0x3bc/0x5f0 net/tipc/netlink_compat.c:393
 tipc_nl_compat_handle net/tipc/netlink_compat.c:-1 [inline]
 tipc_nl_compat_recv+0x83c/0xbe0 net/tipc/netlink_compat.c:1321
 genl_family_rcv_msg_doit+0x215/0x300 net/netlink/genetlink.c:1115
 genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
 genl_rcv_msg+0x60e/0x790 net/netlink/genetlink.c:1210
 netlink_rcv_skb+0x208/0x470 net/netlink/af_netlink.c:2552
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
 netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
 netlink_unicast+0x846/0xa10 net/netlink/af_netlink.c:1346
 netlink_sendmsg+0x805/0xb30 net/netlink/af_netlink.c:1896
 sock_sendmsg_nosec net/socket.c:714 [inline]
 __sock_sendmsg+0x21c/0x270 net/socket.c:729
 ____sys_sendmsg+0x508/0x820 net/socket.c:2614
 ___sys_sendmsg+0x21f/0x2a0 net/socket.c:2668
 __sys_sendmsg net/socket.c:2700 [inline]
 __do_sys_sendmsg net/socket.c:2705 [inline]
 __se_sys_sendmsg net/socket.c:2703 [inline]
 __x64_sys_sendmsg+0x1a1/0x260 net/socket.c:2703
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 6088:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3e/0x80 mm/kasan/common.c:68
 kasan_save_free_info+0x46/0x50 mm/kasan/generic.c:576
 poison_slab_object mm/kasan/common.c:243 [inline]
 __kasan_slab_free+0x5b/0x80 mm/kasan/common.c:275
 kasan_slab_free include/linux/kasan.h:233 [inline]
 slab_free_hook mm/slub.c:2422 [inline]
 slab_free mm/slub.c:4695 [inline]
 kfree+0x195/0x550 mm/slub.c:4894
 tipc_l2_device_event+0x380/0x650 net/tipc/bearer.c:-1
 notifier_call_chain+0x1b3/0x3e0 kernel/notifier.c:85
 call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
 call_netdevice_notifiers net/core/dev.c:2281 [inline]
 unregister_netdevice_many_notify+0x14d7/0x1fe0 net/core/dev.c:12166
 unregister_netdevice_many net/core/dev.c:12229 [inline]
 unregister_netdevice_queue+0x33c/0x380 net/core/dev.c:12073
 unregister_netdevice include/linux/netdevice.h:3385 [inline]
 __tun_detach+0xe4d/0x1620 drivers/net/tun.c:621
 tun_detach drivers/net/tun.c:637 [inline]
 tun_chr_close+0x10d/0x1c0 drivers/net/tun.c:3433
 __fput+0x458/0xa80 fs/file_table.c:468
 task_work_run+0x1d4/0x260 kernel/task_work.c:227
 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
 exit_to_user_mode_loop+0xec/0x110 kernel/entry/common.c:43
 exit_to_user_mode_prepare include/linux/irq-entry-common.h:225 [inline]
 syscall_exit_to_user_mode_work include/linux/entry-common.h:175 [inline]
 syscall_exit_to_user_mode include/linux/entry-common.h:210 [inline]
 do_syscall_64+0x2bd/0x3b0 arch/x86/entry/syscall_64.c:100
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: 46cb01eeeb86 ("tipc: update mon's self addr when node addr generated")
Reported-by: syzbot+d7dad7fd4b3921104957@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/690c323a.050a0220.baf87.007f.GAE@google.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/tipc/monitor.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index 572b79bf76ce..46c8814c3ee6 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -711,6 +711,8 @@ void tipc_mon_reinit_self(struct net *net)
 	struct tipc_monitor *mon;
 	int bearer_id;
 
+	rtnl_lock();
+
 	for (bearer_id = 0; bearer_id < MAX_BEARERS; bearer_id++) {
 		mon = tipc_monitor(net, bearer_id);
 		if (!mon)
@@ -720,6 +722,8 @@ void tipc_mon_reinit_self(struct net *net)
 			mon->self->addr = tipc_own_addr(net);
 		write_unlock_bh(&mon->lock);
 	}
+
+	rtnl_unlock();
 }
 
 int tipc_nl_monitor_set_threshold(struct net *net, u32 cluster_size)
-- 
2.51.2.1026.g39e6a42477-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [syzbot ci] Re: tipc: Fix use-after-free in tipc_mon_reinit_self().
  2025-11-06  5:32 [PATCH v1 net] tipc: Fix use-after-free in tipc_mon_reinit_self() Kuniyuki Iwashima
@ 2025-11-06  9:38 ` syzbot ci
  2025-11-06 17:59   ` Kuniyuki Iwashima
  0 siblings, 1 reply; 7+ messages in thread
From: syzbot ci @ 2025-11-06  9:38 UTC (permalink / raw)
  To: davem, edumazet, hoang.h.le, horms, jmaloy, kuba, kuni1840,
	kuniyu, netdev, pabeni, syzbot, tipc-discussion
  Cc: syzbot, syzkaller-bugs

syzbot ci has tested the following series

[v1] tipc: Fix use-after-free in tipc_mon_reinit_self().
https://lore.kernel.org/all/20251106053309.401275-1-kuniyu@google.com
* [PATCH v1 net] tipc: Fix use-after-free in tipc_mon_reinit_self().

and found the following issue:
possible deadlock in tipc_mon_reinit_self

Full report is available here:
https://ci.syzbot.org/series/bfabf013-65e3-4ca9-8f54-0c7eef8be01a

***

possible deadlock in tipc_mon_reinit_self

tree:      net
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
base:      3d18a84eddde169d6dbf3c72cc5358b988c347d0
arch:      amd64
compiler:  Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
config:    https://ci.syzbot.org/builds/b2774856-e331-420e-a340-5107ec4b06f9/config
C repro:   https://ci.syzbot.org/findings/1f0a4298-b797-4217-8d6d-15f98c0ffd38/c_repro
syz repro: https://ci.syzbot.org/findings/1f0a4298-b797-4217-8d6d-15f98c0ffd38/syz_repro

tipc: Started in network mode
tipc: Node identity 4, cluster identity 4711
tipc: Node number set to 4
============================================
WARNING: possible recursive locking detected
syzkaller #0 Not tainted
--------------------------------------------
syz.0.17/5963 is trying to acquire lock:
ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: tipc_mon_reinit_self+0x25/0x360 net/tipc/monitor.c:714

but task is already holding lock:
ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: __tipc_nl_compat_doit net/tipc/netlink_compat.c:358 [inline]
ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: tipc_nl_compat_doit+0x1fd/0x5f0 net/tipc/netlink_compat.c:393

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(rtnl_mutex);
  lock(rtnl_mutex);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

3 locks held by syz.0.17/5963:
 #0: ffffffff8f331050 (cb_lock){++++}-{4:4}, at: genl_rcv+0x19/0x40 net/netlink/genetlink.c:1218
 #1: ffffffff8f330e68 (genl_mutex){+.+.}-{4:4}, at: genl_lock net/netlink/genetlink.c:35 [inline]
 #1: ffffffff8f330e68 (genl_mutex){+.+.}-{4:4}, at: genl_op_lock net/netlink/genetlink.c:60 [inline]
 #1: ffffffff8f330e68 (genl_mutex){+.+.}-{4:4}, at: genl_rcv_msg+0x10d/0x790 net/netlink/genetlink.c:1209
 #2: ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: __tipc_nl_compat_doit net/tipc/netlink_compat.c:358 [inline]
 #2: ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: tipc_nl_compat_doit+0x1fd/0x5f0 net/tipc/netlink_compat.c:393

stack backtrace:
CPU: 1 UID: 0 PID: 5963 Comm: syz.0.17 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
Call Trace:
 <TASK>
 dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
 print_deadlock_bug+0x28b/0x2a0 kernel/locking/lockdep.c:3041
 check_deadlock kernel/locking/lockdep.c:3093 [inline]
 validate_chain+0x1a3f/0x2140 kernel/locking/lockdep.c:3895
 __lock_acquire+0xab9/0xd20 kernel/locking/lockdep.c:5237
 lock_acquire+0x120/0x360 kernel/locking/lockdep.c:5868
 __mutex_lock_common kernel/locking/mutex.c:598 [inline]
 __mutex_lock+0x187/0x1350 kernel/locking/mutex.c:760
 tipc_mon_reinit_self+0x25/0x360 net/tipc/monitor.c:714
 tipc_net_finalize+0x115/0x190 net/tipc/net.c:140
 tipc_net_init+0x104/0x190 net/tipc/net.c:122
 __tipc_nl_net_set+0x3b9/0x5a0 net/tipc/net.c:263
 __tipc_nl_compat_doit net/tipc/netlink_compat.c:371 [inline]
 tipc_nl_compat_doit+0x3bc/0x5f0 net/tipc/netlink_compat.c:393
 tipc_nl_compat_handle net/tipc/netlink_compat.c:-1 [inline]
 tipc_nl_compat_recv+0x83c/0xbe0 net/tipc/netlink_compat.c:1321
 genl_family_rcv_msg_doit+0x215/0x300 net/netlink/genetlink.c:1115
 genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
 genl_rcv_msg+0x60e/0x790 net/netlink/genetlink.c:1210
 netlink_rcv_skb+0x208/0x470 net/netlink/af_netlink.c:2552
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
 netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
 netlink_unicast+0x82f/0x9e0 net/netlink/af_netlink.c:1346
 netlink_sendmsg+0x805/0xb30 net/netlink/af_netlink.c:1896
 sock_sendmsg_nosec net/socket.c:727 [inline]
 __sock_sendmsg+0x21c/0x270 net/socket.c:742
 ____sys_sendmsg+0x505/0x830 net/socket.c:2630
 ___sys_sendmsg+0x21f/0x2a0 net/socket.c:2684
 __sys_sendmsg net/socket.c:2716 [inline]
 __do_sys_sendmsg net/socket.c:2721 [inline]
 __se_sys_sendmsg net/socket.c:2719 [inline]
 __x64_sys_sendmsg+0x19b/0x260 net/socket.c:2719
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0xfa0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f13d8b8efc9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 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 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff77cccbf8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f13d8de5fa0 RCX: 00007f13d8b8efc9
RDX: 0000000000000000 RSI: 00002000000001c0 RDI: 0000000000000003
RBP: 00007f13d8c11f91 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f13d8de5fa0 R14: 00007f13d8de5fa0 R15: 0000000000000003
 </TASK>


***

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [syzbot ci] Re: tipc: Fix use-after-free in tipc_mon_reinit_self().
  2025-11-06  9:38 ` [syzbot ci] " syzbot ci
@ 2025-11-06 17:59   ` Kuniyuki Iwashima
  2025-11-06 22:30     ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-11-06 17:59 UTC (permalink / raw)
  To: syzbot+cif2d6d318f7e85f0b
  Cc: davem, edumazet, hoang.h.le, horms, jmaloy, kuba, kuni1840,
	kuniyu, netdev, pabeni, syzbot, syzbot, syzkaller-bugs,
	tipc-discussion

From: syzbot ci <syzbot+cif2d6d318f7e85f0b@syzkaller.appspotmail.com>
Date: Thu, 06 Nov 2025 01:38:49 -0800
> syzbot ci has tested the following series
> 
> [v1] tipc: Fix use-after-free in tipc_mon_reinit_self().
> https://lore.kernel.org/all/20251106053309.401275-1-kuniyu@google.com
> * [PATCH v1 net] tipc: Fix use-after-free in tipc_mon_reinit_self().
> 
> and found the following issue:
> possible deadlock in tipc_mon_reinit_self
> 
> Full report is available here:
> https://ci.syzbot.org/series/bfabf013-65e3-4ca9-8f54-0c7eef8be01a
> 
> ***
> 
> possible deadlock in tipc_mon_reinit_self
> 
> tree:      net
> URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
> base:      3d18a84eddde169d6dbf3c72cc5358b988c347d0
> arch:      amd64
> compiler:  Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> config:    https://ci.syzbot.org/builds/b2774856-e331-420e-a340-5107ec4b06f9/config
> C repro:   https://ci.syzbot.org/findings/1f0a4298-b797-4217-8d6d-15f98c0ffd38/c_repro
> syz repro: https://ci.syzbot.org/findings/1f0a4298-b797-4217-8d6d-15f98c0ffd38/syz_repro
> 
> tipc: Started in network mode
> tipc: Node identity 4, cluster identity 4711
> tipc: Node number set to 4
> ============================================
> WARNING: possible recursive locking detected
> syzkaller #0 Not tainted
> --------------------------------------------
> syz.0.17/5963 is trying to acquire lock:
> ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: tipc_mon_reinit_self+0x25/0x360 net/tipc/monitor.c:714
> 
> but task is already holding lock:
> ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: __tipc_nl_compat_doit net/tipc/netlink_compat.c:358 [inline]
> ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: tipc_nl_compat_doit+0x1fd/0x5f0 net/tipc/netlink_compat.c:393
> 
> other info that might help us debug this:
>  Possible unsafe locking scenario:
> 
>        CPU0
>        ----
>   lock(rtnl_mutex);
>   lock(rtnl_mutex);
> 
>  *** DEADLOCK ***
> 
>  May be due to missing lock nesting notation
> 
> 3 locks held by syz.0.17/5963:
>  #0: ffffffff8f331050 (cb_lock){++++}-{4:4}, at: genl_rcv+0x19/0x40 net/netlink/genetlink.c:1218
>  #1: ffffffff8f330e68 (genl_mutex){+.+.}-{4:4}, at: genl_lock net/netlink/genetlink.c:35 [inline]
>  #1: ffffffff8f330e68 (genl_mutex){+.+.}-{4:4}, at: genl_op_lock net/netlink/genetlink.c:60 [inline]
>  #1: ffffffff8f330e68 (genl_mutex){+.+.}-{4:4}, at: genl_rcv_msg+0x10d/0x790 net/netlink/genetlink.c:1209
>  #2: ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: __tipc_nl_compat_doit net/tipc/netlink_compat.c:358 [inline]
>  #2: ffffffff8f2cb1c8 (rtnl_mutex){+.+.}-{4:4}, at: tipc_nl_compat_doit+0x1fd/0x5f0 net/tipc/netlink_compat.c:393
> 
> stack backtrace:
> CPU: 1 UID: 0 PID: 5963 Comm: syz.0.17 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
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
>  print_deadlock_bug+0x28b/0x2a0 kernel/locking/lockdep.c:3041
>  check_deadlock kernel/locking/lockdep.c:3093 [inline]
>  validate_chain+0x1a3f/0x2140 kernel/locking/lockdep.c:3895
>  __lock_acquire+0xab9/0xd20 kernel/locking/lockdep.c:5237
>  lock_acquire+0x120/0x360 kernel/locking/lockdep.c:5868
>  __mutex_lock_common kernel/locking/mutex.c:598 [inline]
>  __mutex_lock+0x187/0x1350 kernel/locking/mutex.c:760
>  tipc_mon_reinit_self+0x25/0x360 net/tipc/monitor.c:714
>  tipc_net_finalize+0x115/0x190 net/tipc/net.c:140
>  tipc_net_init+0x104/0x190 net/tipc/net.c:122
>  __tipc_nl_net_set+0x3b9/0x5a0 net/tipc/net.c:263

I missed another path calling tipc_net_finalize under RTNL.

I'll change v2 this way.

---8<---
diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index 46c8814c3ee6..be1e51efc445 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -706,12 +706,13 @@ void tipc_mon_delete(struct net *net, int bearer_id)
 	kfree(mon);
 }
 
-void tipc_mon_reinit_self(struct net *net)
+void tipc_mon_reinit_self(struct net *net, bool rtnl_held)
 {
 	struct tipc_monitor *mon;
 	int bearer_id;
 
-	rtnl_lock();
+	if (!rtnl_held)
+		rtnl_lock();
 
 	for (bearer_id = 0; bearer_id < MAX_BEARERS; bearer_id++) {
 		mon = tipc_monitor(net, bearer_id);
@@ -723,7 +724,8 @@ void tipc_mon_reinit_self(struct net *net)
 		write_unlock_bh(&mon->lock);
 	}
 
-	rtnl_unlock();
+	if (!rtnl_held)
+		rtnl_unlock();
 }
 
 int tipc_nl_monitor_set_threshold(struct net *net, u32 cluster_size)
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 0e95572e56b4..56527f6f548c 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -119,11 +119,11 @@ int tipc_net_init(struct net *net, u8 *node_id, u32 addr)
 	if (node_id)
 		tipc_set_node_id(net, node_id);
 	if (addr)
-		tipc_net_finalize(net, addr);
+		tipc_net_finalize(net, addr, true);
 	return 0;
 }
 
-static void tipc_net_finalize(struct net *net, u32 addr)
+static void tipc_net_finalize(struct net *net, u32 addr, bool rtnl_held)
 {
 	struct tipc_net *tn = tipc_net(net);
 	struct tipc_socket_addr sk = {0, addr};
@@ -137,7 +137,7 @@ static void tipc_net_finalize(struct net *net, u32 addr)
 	tipc_set_node_addr(net, addr);
 	tipc_named_reinit(net);
 	tipc_sk_reinit(net);
-	tipc_mon_reinit_self(net);
+	tipc_mon_reinit_self(net, rtnl_held);
 	tipc_nametbl_publish(net, &ua, &sk, addr);
 }
 
@@ -145,7 +145,7 @@ void tipc_net_finalize_work(struct work_struct *work)
 {
 	struct tipc_net *tn = container_of(work, struct tipc_net, work);
 
-	tipc_net_finalize(tipc_link_net(tn->bcl), tn->trial_addr);
+	tipc_net_finalize(tipc_link_net(tn->bcl), tn->trial_addr, false);
 }
 
 void tipc_net_stop(struct net *net)
---8<---

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [syzbot ci] Re: tipc: Fix use-after-free in tipc_mon_reinit_self().
  2025-11-06 17:59   ` Kuniyuki Iwashima
@ 2025-11-06 22:30     ` Jakub Kicinski
  2025-11-06 22:37       ` Kuniyuki Iwashima
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2025-11-06 22:30 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: syzbot+cif2d6d318f7e85f0b, davem, edumazet, hoang.h.le, horms,
	jmaloy, kuni1840, netdev, pabeni, syzbot, syzbot, syzkaller-bugs,
	tipc-discussion

On Thu,  6 Nov 2025 17:59:17 +0000 Kuniyuki Iwashima wrote:
> -void tipc_mon_reinit_self(struct net *net)
> +void tipc_mon_reinit_self(struct net *net, bool rtnl_held)
>  {
>  	struct tipc_monitor *mon;
>  	int bearer_id;
>  
> -	rtnl_lock();
> +	if (!rtnl_held)
> +		rtnl_lock();

I haven't looked closely but for the record conditional locking 
is generally considered to be poor code design. Extract the body
into a __tipc_mon_reinit_self() helper and call that when lock 
is already held? And:

void tipc_mon_reinit_self(struct net *net)
{
	rtnl_lock();
	__tipc_mon_reinit_self(net);
	rtnl_unlock();
}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [syzbot ci] Re: tipc: Fix use-after-free in tipc_mon_reinit_self().
  2025-11-06 22:30     ` Jakub Kicinski
@ 2025-11-06 22:37       ` Kuniyuki Iwashima
  2025-11-06 22:39         ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-11-06 22:37 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: syzbot+cif2d6d318f7e85f0b, davem, edumazet, hoang.h.le, horms,
	jmaloy, kuni1840, netdev, pabeni, syzbot, syzbot, syzkaller-bugs,
	tipc-discussion

On Thu, Nov 6, 2025 at 2:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu,  6 Nov 2025 17:59:17 +0000 Kuniyuki Iwashima wrote:
> > -void tipc_mon_reinit_self(struct net *net)
> > +void tipc_mon_reinit_self(struct net *net, bool rtnl_held)
> >  {
> >       struct tipc_monitor *mon;
> >       int bearer_id;
> >
> > -     rtnl_lock();
> > +     if (!rtnl_held)
> > +             rtnl_lock();
>
> I haven't looked closely but for the record conditional locking
> is generally considered to be poor code design. Extract the body
> into a __tipc_mon_reinit_self() helper and call that when lock
> is already held? And:
>
> void tipc_mon_reinit_self(struct net *net)
> {
>         rtnl_lock();
>         __tipc_mon_reinit_self(net);
>         rtnl_unlock();
> }

That's much cleaner, I'll use this.

Thanks, Jakub!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [syzbot ci] Re: tipc: Fix use-after-free in tipc_mon_reinit_self().
  2025-11-06 22:37       ` Kuniyuki Iwashima
@ 2025-11-06 22:39         ` Jakub Kicinski
  2025-11-06 22:51           ` Kuniyuki Iwashima
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2025-11-06 22:39 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: syzbot+cif2d6d318f7e85f0b, davem, edumazet, hoang.h.le, horms,
	jmaloy, kuni1840, netdev, pabeni, syzbot, syzbot, syzkaller-bugs,
	tipc-discussion

On Thu, 6 Nov 2025 14:37:10 -0800 Kuniyuki Iwashima wrote:
> > On Thu,  6 Nov 2025 17:59:17 +0000 Kuniyuki Iwashima wrote:  
> > > -void tipc_mon_reinit_self(struct net *net)
> > > +void tipc_mon_reinit_self(struct net *net, bool rtnl_held)
> > >  {
> > >       struct tipc_monitor *mon;
> > >       int bearer_id;
> > >
> > > -     rtnl_lock();
> > > +     if (!rtnl_held)
> > > +             rtnl_lock();  
> >
> > I haven't looked closely but for the record conditional locking
> > is generally considered to be poor code design. Extract the body
> > into a __tipc_mon_reinit_self() helper and call that when lock
> > is already held? And:
> >
> > void tipc_mon_reinit_self(struct net *net)
> > {
> >         rtnl_lock();
> >         __tipc_mon_reinit_self(net);
> >         rtnl_unlock();
> > }  
> 
> That's much cleaner, I'll use this.

After sending I realized you probably want to do this wrapping around
tipc_net_finalize(), otherwise we'd just be shifting the conditional.
But you get the point.. :)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [syzbot ci] Re: tipc: Fix use-after-free in tipc_mon_reinit_self().
  2025-11-06 22:39         ` Jakub Kicinski
@ 2025-11-06 22:51           ` Kuniyuki Iwashima
  0 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-11-06 22:51 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: syzbot+cif2d6d318f7e85f0b, davem, edumazet, hoang.h.le, horms,
	jmaloy, kuni1840, netdev, pabeni, syzbot, syzbot, syzkaller-bugs,
	tipc-discussion

On Thu, Nov 6, 2025 at 2:39 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 6 Nov 2025 14:37:10 -0800 Kuniyuki Iwashima wrote:
> > > On Thu,  6 Nov 2025 17:59:17 +0000 Kuniyuki Iwashima wrote:
> > > > -void tipc_mon_reinit_self(struct net *net)
> > > > +void tipc_mon_reinit_self(struct net *net, bool rtnl_held)
> > > >  {
> > > >       struct tipc_monitor *mon;
> > > >       int bearer_id;
> > > >
> > > > -     rtnl_lock();
> > > > +     if (!rtnl_held)
> > > > +             rtnl_lock();
> > >
> > > I haven't looked closely but for the record conditional locking
> > > is generally considered to be poor code design. Extract the body
> > > into a __tipc_mon_reinit_self() helper and call that when lock
> > > is already held? And:
> > >
> > > void tipc_mon_reinit_self(struct net *net)
> > > {
> > >         rtnl_lock();
> > >         __tipc_mon_reinit_self(net);
> > >         rtnl_unlock();
> > > }
> >
> > That's much cleaner, I'll use this.
>
> After sending I realized you probably want to do this wrapping around
> tipc_net_finalize(), otherwise we'd just be shifting the conditional.
> But you get the point.. :)

Yes, will wrap it in tipc_net_finalize_work() :)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-06 22:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-06  5:32 [PATCH v1 net] tipc: Fix use-after-free in tipc_mon_reinit_self() Kuniyuki Iwashima
2025-11-06  9:38 ` [syzbot ci] " syzbot ci
2025-11-06 17:59   ` Kuniyuki Iwashima
2025-11-06 22:30     ` Jakub Kicinski
2025-11-06 22:37       ` Kuniyuki Iwashima
2025-11-06 22:39         ` Jakub Kicinski
2025-11-06 22:51           ` Kuniyuki Iwashima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).