public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
@ 2026-03-30 13:03 Kai Zen
  2026-03-31  3:46 ` Tung Quang Nguyen
  2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
  0 siblings, 2 replies; 7+ messages in thread
From: Kai Zen @ 2026-03-30 13:03 UTC (permalink / raw)
  To: netdev; +Cc: stable, jmaloy

CVE-2025-40280 fixed tipc_mon_reinit_self() accessing monitors[] from a
workqueue without RTNL.  That patch closed the workqueue path by adding
rtnl_lock() around the call.

However, three additional functions in the same subsystem access
tipc_net->monitors[] from softirq context with no RCU protection at all:

  tipc_mon_peer_up()    - called from tipc_node_write_unlock()
  tipc_mon_peer_down()  - called from tipc_node_write_unlock()
  tipc_mon_remove_peer() - called from tipc_node_link_down()

These three are invoked from the packet receive path (tipc_rcv ->
tipc_node_write_unlock / tipc_node_link_down) and hold only the per-node
rwlock, not RTNL.

Concurrently, bearer_disable() -- which always holds RTNL per its own
inline documentation -- calls tipc_mon_delete(), which:

  1. acquires mon->lock
  2. sets tn->monitors[bearer_id] = NULL
  3. frees all peer entries
  4. releases mon->lock
  5. calls kfree(mon)  <-- no synchronize_rcu()

The race is structural: there is no shared lock between the data-path
reader (which reads monitors[id] then acquires mon->lock) and the
teardown path (which acquires mon->lock, NULLs the slot, then frees).
A softirq thread can read a non-NULL mon pointer, get preempted, and
resume after kfree(mon) has run on another CPU, then call
write_lock_bh(&mon->lock) on freed memory:

  CPU 0 (softirq / tipc_rcv)          CPU 1 (RTNL / bearer_disable)
  tipc_mon_peer_up()
    mon = tipc_monitor(net, id)
    [mon is non-NULL]
                                       tipc_mon_delete()
                                         write_lock_bh(&mon->lock)
                                         tn->monitors[id] = NULL
                                         ...
                                         write_unlock_bh(&mon->lock)
                                         kfree(mon)
    write_lock_bh(&mon->lock)  <-- UAF

The fix mirrors the existing bearer_list[] pattern in the same module:
convert monitors[] to __rcu, use rcu_assign_pointer() on creation,
RCU_INIT_POINTER() + synchronize_rcu() on deletion (before the kfree),
and the appropriate rcu_dereference_bh() vs rtnl_dereference() variant
at each read site depending on execution context.

synchronize_rcu() in tipc_mon_delete() is placed after the
write_unlock_bh() and before timer_shutdown_sync() + kfree() to ensure
all softirq-context readers that already observed the old pointer have
completed before the memory is freed.

Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
Cc: stable@vger.kernel.org
Signed-off-by: Kai Aizen <kai.aizen.dev@gmail.com>
---
v2: Resubmit targeting mainline via netdev per stable-kernel-rules (Option 1).
    No code changes from v1.

 net/tipc/core.h    |  2 +-
 net/tipc/monitor.c | 51 +++++++++++++++++++++++++++++++++--------------
 2 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/net/tipc/core.h b/net/tipc/core.h
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -109,7 +109,7 @@
  u32 num_links;
  /* Neighbor monitoring list */
- struct tipc_monitor *monitors[MAX_BEARERS];
+ struct tipc_monitor __rcu *monito[MAX_BEARERS];
 rs
+

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

* RE: [PATCH net v2] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
  2026-03-30 13:03 [PATCH net v2] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown Kai Zen
@ 2026-03-31  3:46 ` Tung Quang Nguyen
  2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
  1 sibling, 0 replies; 7+ messages in thread
From: Tung Quang Nguyen @ 2026-03-31  3:46 UTC (permalink / raw)
  To: Kai Zen; +Cc: stable@vger.kernel.org, jmaloy@redhat.com, netdev@vger.kernel.org

>Subject: [PATCH net v2] tipc: fix UAF race in
>tipc_mon_peer_up/down/remove_peer vs bearer teardown
>
>CVE-2025-40280 fixed tipc_mon_reinit_self() accessing monitors[] from a
>workqueue without RTNL.  That patch closed the workqueue path by adding
>rtnl_lock() around the call.
>
>However, three additional functions in the same subsystem access tipc_net-
>>monitors[] from softirq context with no RCU protection at all:
>
>  tipc_mon_peer_up()    - called from tipc_node_write_unlock()
>  tipc_mon_peer_down()  - called from tipc_node_write_unlock()
>  tipc_mon_remove_peer() - called from tipc_node_link_down()
>
>These three are invoked from the packet receive path (tipc_rcv ->
>tipc_node_write_unlock / tipc_node_link_down) and hold only the per-node
>rwlock, not RTNL.
>
>Concurrently, bearer_disable() -- which always holds RTNL per its own inline
>documentation -- calls tipc_mon_delete(), which:
>
>  1. acquires mon->lock
>  2. sets tn->monitors[bearer_id] = NULL
>  3. frees all peer entries
>  4. releases mon->lock
>  5. calls kfree(mon)  <-- no synchronize_rcu()
>
>The race is structural: there is no shared lock between the data-path reader
>(which reads monitors[id] then acquires mon->lock) and the teardown path
>(which acquires mon->lock, NULLs the slot, then frees).
>A softirq thread can read a non-NULL mon pointer, get preempted, and resume
>after kfree(mon) has run on another CPU, then call
>write_lock_bh(&mon->lock) on freed memory:
>
>  CPU 0 (softirq / tipc_rcv)          CPU 1 (RTNL / bearer_disable)
>  tipc_mon_peer_up()
>    mon = tipc_monitor(net, id)
>    [mon is non-NULL]
>                                       tipc_mon_delete()
>                                         write_lock_bh(&mon->lock)
>                                         tn->monitors[id] = NULL
>                                         ...
>                                         write_unlock_bh(&mon->lock)
>                                         kfree(mon)
>    write_lock_bh(&mon->lock)  <-- UAF
>
Can you reproduce above scenario and capture the stack trace when UAF happens ?

>The fix mirrors the existing bearer_list[] pattern in the same module:
>convert monitors[] to __rcu, use rcu_assign_pointer() on creation,
>RCU_INIT_POINTER() + synchronize_rcu() on deletion (before the kfree), and
>the appropriate rcu_dereference_bh() vs rtnl_dereference() variant at each
>read site depending on execution context.
>
>synchronize_rcu() in tipc_mon_delete() is placed after the
>write_unlock_bh() and before timer_shutdown_sync() + kfree() to ensure all
>softirq-context readers that already observed the old pointer have completed
>before the memory is freed.
>
Not sure why your patch does not implement your above solution. I see only one change in core.h.
>Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
>Cc: stable@vger.kernel.org
>Signed-off-by: Kai Aizen <kai.aizen.dev@gmail.com>
>---
>v2: Resubmit targeting mainline via netdev per stable-kernel-rules (Option 1).
>    No code changes from v1.
>
> net/tipc/core.h    |  2 +-
> net/tipc/monitor.c | 51 +++++++++++++++++++++++++++++++++--------------
> 2 files changed, 37 insertions(+), 16 deletions(-)
>
>diff --git a/net/tipc/core.h b/net/tipc/core.h
>--- a/net/tipc/core.h
>+++ b/net/tipc/core.h
>@@ -109,7 +109,7 @@
>  u32 num_links;
>  /* Neighbor monitoring list */
>- struct tipc_monitor *monitors[MAX_BEARERS];
>+ struct tipc_monitor __rcu *monito[MAX_BEARERS];
> rs
>+


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

* [PATCH net v3] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
  2026-03-30 13:03 [PATCH net v2] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown Kai Zen
  2026-03-31  3:46 ` Tung Quang Nguyen
@ 2026-04-30 15:26 ` SnailSploit | Kai Aizen
  2026-04-30 15:40   ` SnailSploit | Kai Aizen
                     ` (3 more replies)
  1 sibling, 4 replies; 7+ messages in thread
From: SnailSploit | Kai Aizen @ 2026-04-30 15:26 UTC (permalink / raw)
  To: netdev
  Cc: stable, jmaloy, ying.xue, kuba, pabeni, tipc-discussion,
	tung.q.nguyen, lkp, oe-kbuild-all, syzkaller-bugs,
	SnailSploit | Kai Aizen, syzbot ci

From: "SnailSploit | Kai Aizen" <95986478+SnailSploit@users.noreply.github.com>

CVE-2025-40280 fixed tipc_mon_reinit_self() accessing monitors[] from a
workqueue without RTNL.  That patch closed the workqueue path by adding
rtnl_lock() around the call.

However, three additional functions in the same subsystem access
tipc_net->monitors[] from softirq context with no RCU protection at all:

  tipc_mon_peer_up()     - called from tipc_node_write_unlock()
  tipc_mon_peer_down()   - called from tipc_node_write_unlock()
  tipc_mon_remove_peer() - called from tipc_node_link_down()

These are invoked from the packet receive path (tipc_rcv ->
tipc_node_write_unlock / tipc_node_link_down) and hold only the per-node
rwlock, not RTNL.

Concurrently, bearer_disable() -- which always holds RTNL -- calls
tipc_mon_delete(), which sets tn->monitors[bearer_id] = NULL and then
kfree(mon) without an RCU grace period. A softirq reader can observe
the non-NULL slot, take a reference, get preempted, and resume after
kfree(mon) on another CPU, dereferencing freed memory.

Convert monitors[] to __rcu, use rcu_assign_pointer() on creation,
RCU_INIT_POINTER() + synchronize_rcu() on deletion before kfree(), and
the appropriate dereference variant at each read site:

  - tipc_monitor() returns rcu_dereference_bh(...) for softirq callers
    (tipc_mon_peer_up/down/remove_peer/rcv/prep/get_state).
  - tipc_monitor_rtnl() returns rtnl_dereference(...) for RTNL-held
    callers (tipc_mon_delete via bearer_disable, tipc_mon_reinit_self
    via tipc_net_finalize_work which wraps in rtnl_lock(), and the
    netlink dump handlers tipc_nl_add_monitor_peer /
    __tipc_nl_add_monitor).

Also, get_self() was a thin wrapper over tipc_monitor() + ->self deref,
duplicating the RCU-checked load that callers already perform on entry.
With monitors[] becoming __rcu, get_self()'s use of tipc_monitor()
generates a lockdep splat in tipc_mon_delete() (RTNL context) because
the inner load is rcu_dereference_bh().  syzbot CI reported this on
v1/v2 of this patch:

  WARNING: suspicious RCU usage in tipc_mon_delete
  net/tipc/monitor.c:108 suspicious rcu_dereference_check() usage!
  ...
  tipc_monitor_rcu_bh+0xf5/0x110  net/tipc/monitor.c:108
  get_self                        net/tipc/monitor.c:209
  tipc_mon_delete+0x10b/0x4d0     net/tipc/monitor.c:704

Drop get_self() entirely.  Each existing caller already has a valid
mon pointer from its initial RCU-correct load, and mon->self is the
result get_self() was returning.  Replace each "self = get_self(...)"
with "self = mon->self;".  This both removes the duplicate dereference
and fixes the lockdep splat.

synchronize_rcu() in tipc_mon_delete() is placed after
write_unlock_bh() and before timer_shutdown_sync() + kfree() so all
softirq readers that already observed the old pointer have completed
before the memory is freed.

Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
Cc: stable@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604301148.jfXKC9HF-lkp@intel.com/
Reported-by: syzbot ci <syzbot+ci779e8ed86620f383@syzkaller.appspotmail.com>
Closes: https://ci.syzbot.org/series/6267bc07-4172-4821-b3e5-dac381479d9d
Signed-off-by: SnailSploit | Kai Aizen <95986478+SnailSploit@users.noreply.github.com>
---
 net/tipc/core.h    |  2 +-
 net/tipc/monitor.c | 42 +++++++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/net/tipc/core.h b/net/tipc/core.h
index 9ce5f9ff6..cd582f7a2 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -109,7 +109,7 @@ struct tipc_net {
 	u32 num_links;
 
 	/* Neighbor monitoring list */
-	struct tipc_monitor *monitors[MAX_BEARERS];
+	struct tipc_monitor __rcu *monitors[MAX_BEARERS];
 	int mon_threshold;
 
 	/* Bearer list */
diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index a94b9b36a..0095a62ae 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -99,7 +99,14 @@ struct tipc_monitor {
 
 static struct tipc_monitor *tipc_monitor(struct net *net, int bearer_id)
 {
-	return tipc_net(net)->monitors[bearer_id];
+	return rcu_dereference_bh(tipc_net(net)->monitors[bearer_id]);
+}
+
+/* tipc_monitor_rtnl - dereference monitors[] from RTNL-held control path. */
+static struct tipc_monitor * __maybe_unused
+tipc_monitor_rtnl(struct net *net, int bearer_id)
+{
+	return rtnl_dereference(tipc_net(net)->monitors[bearer_id]);
 }
 
 const int tipc_max_domain_size = sizeof(struct tipc_mon_domain);
@@ -192,13 +199,6 @@ static struct tipc_peer *get_peer(struct tipc_monitor *mon, u32 addr)
 	return NULL;
 }
 
-static struct tipc_peer *get_self(struct net *net, int bearer_id)
-{
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
-
-	return mon->self;
-}
-
 static inline bool tipc_mon_is_active(struct net *net, struct tipc_monitor *mon)
 {
 	struct tipc_net *tn = tipc_net(net);
@@ -358,7 +358,7 @@ void tipc_mon_remove_peer(struct net *net, u32 addr, int bearer_id)
 	if (!mon)
 		return;
 
-	self = get_self(net, bearer_id);
+	self = mon->self;
 	write_lock_bh(&mon->lock);
 	peer = get_peer(mon, addr);
 	if (!peer)
@@ -422,9 +422,12 @@ static bool tipc_mon_add_peer(struct tipc_monitor *mon, u32 addr,
 void tipc_mon_peer_up(struct net *net, u32 addr, int bearer_id)
 {
 	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
-	struct tipc_peer *self = get_self(net, bearer_id);
+	struct tipc_peer *self;
 	struct tipc_peer *peer, *head;
 
+	if (!mon)
+		return;
+	self = mon->self;
 	write_lock_bh(&mon->lock);
 	peer = get_peer(mon, addr);
 	if (!peer && !tipc_mon_add_peer(mon, addr, &peer))
@@ -449,7 +452,7 @@ void tipc_mon_peer_down(struct net *net, u32 addr, int bearer_id)
 	if (!mon)
 		return;
 
-	self = get_self(net, bearer_id);
+	self = mon->self;
 	write_lock_bh(&mon->lock);
 	peer = get_peer(mon, addr);
 	if (!peer) {
@@ -651,7 +654,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
 	struct tipc_peer *self;
 	struct tipc_mon_domain *dom;
 
-	if (tn->monitors[bearer_id])
+	if (rtnl_dereference(tn->monitors[bearer_id]))
 		return 0;
 
 	mon = kzalloc_obj(*mon, GFP_ATOMIC);
@@ -663,7 +666,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
 		kfree(dom);
 		return -ENOMEM;
 	}
-	tn->monitors[bearer_id] = mon;
+	rcu_assign_pointer(tn->monitors[bearer_id], mon);
 	rwlock_init(&mon->lock);
 	mon->net = net;
 	mon->peer_cnt = 1;
@@ -682,16 +685,16 @@ int tipc_mon_create(struct net *net, int bearer_id)
 void tipc_mon_delete(struct net *net, int bearer_id)
 {
 	struct tipc_net *tn = tipc_net(net);
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
 	struct tipc_peer *self;
 	struct tipc_peer *peer, *tmp;
 
 	if (!mon)
 		return;
 
-	self = get_self(net, bearer_id);
+	self = mon->self;
+	RCU_INIT_POINTER(tn->monitors[bearer_id], NULL);
 	write_lock_bh(&mon->lock);
-	tn->monitors[bearer_id] = NULL;
 	list_for_each_entry_safe(peer, tmp, &self->list, list) {
 		list_del(&peer->list);
 		hlist_del(&peer->hash);
@@ -700,6 +703,7 @@ void tipc_mon_delete(struct net *net, int bearer_id)
 	}
 	mon->self = NULL;
 	write_unlock_bh(&mon->lock);
+	synchronize_rcu();
 	timer_shutdown_sync(&mon->timer);
 	kfree(self->domain);
 	kfree(self);
@@ -712,7 +716,7 @@ void tipc_mon_reinit_self(struct net *net)
 	int bearer_id;
 
 	for (bearer_id = 0; bearer_id < MAX_BEARERS; bearer_id++) {
-		mon = tipc_monitor(net, bearer_id);
+		mon = tipc_monitor_rtnl(net, bearer_id);
 		if (!mon)
 			continue;
 		write_lock_bh(&mon->lock);
@@ -798,7 +802,7 @@ static int __tipc_nl_add_monitor_peer(struct tipc_peer *peer,
 int tipc_nl_add_monitor_peer(struct net *net, struct tipc_nl_msg *msg,
 			     u32 bearer_id, u32 *prev_node)
 {
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
 	struct tipc_peer *peer;
 
 	if (!mon)
@@ -827,7 +831,7 @@ int tipc_nl_add_monitor_peer(struct net *net, struct tipc_nl_msg *msg,
 int __tipc_nl_add_monitor(struct net *net, struct tipc_nl_msg *msg,
 			  u32 bearer_id)
 {
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
 	char bearer_name[TIPC_MAX_BEARER_NAME];
 	struct nlattr *attrs;
 	void *hdr;
-- 
2.43.0


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

* [PATCH net v3] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
  2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
@ 2026-04-30 15:40   ` SnailSploit | Kai Aizen
  2026-05-02  3:35   ` Tung Quang Nguyen
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: SnailSploit | Kai Aizen @ 2026-04-30 15:40 UTC (permalink / raw)
  To: netdev
  Cc: stable, jmaloy, ying.xue, kuba, pabeni, tipc-discussion,
	tung.q.nguyen, lkp, oe-kbuild-all, syzkaller-bugs,
	SnailSploit | Kai Aizen, syzbot ci

From: "SnailSploit | Kai Aizen" <95986478+SnailSploit@users.noreply.github.com>

CVE-2025-40280 fixed tipc_mon_reinit_self() accessing monitors[] from a
workqueue without RTNL.  That patch closed the workqueue path by adding
rtnl_lock() around the call.

However, three additional functions in the same subsystem access
tipc_net->monitors[] from softirq context with no RCU protection at all:

  tipc_mon_peer_up()     - called from tipc_node_write_unlock()
  tipc_mon_peer_down()   - called from tipc_node_write_unlock()
  tipc_mon_remove_peer() - called from tipc_node_link_down()

These are invoked from the packet receive path (tipc_rcv ->
tipc_node_write_unlock / tipc_node_link_down) and hold only the per-node
rwlock, not RTNL.

Concurrently, bearer_disable() -- which always holds RTNL -- calls
tipc_mon_delete(), which sets tn->monitors[bearer_id] = NULL and then
kfree(mon) without an RCU grace period. A softirq reader can observe
the non-NULL slot, take a reference, get preempted, and resume after
kfree(mon) on another CPU, dereferencing freed memory.

Convert monitors[] to __rcu, use rcu_assign_pointer() on creation,
RCU_INIT_POINTER() + synchronize_rcu() on deletion before kfree(), and
the appropriate dereference variant at each read site:

  - tipc_monitor() returns rcu_dereference_bh(...) for softirq callers
    (tipc_mon_peer_up/down/remove_peer/rcv/prep/get_state).
  - tipc_monitor_rtnl() returns rtnl_dereference(...) for RTNL-held
    callers (tipc_mon_delete via bearer_disable, tipc_mon_reinit_self
    via tipc_net_finalize_work which wraps in rtnl_lock(), and the
    netlink dump handlers tipc_nl_add_monitor_peer /
    __tipc_nl_add_monitor).

Also, get_self() was a thin wrapper over tipc_monitor() + ->self deref,
duplicating the RCU-checked load that callers already perform on entry.
With monitors[] becoming __rcu, get_self()'s use of tipc_monitor()
generates a lockdep splat in tipc_mon_delete() (RTNL context) because
the inner load is rcu_dereference_bh().  syzbot CI reported this on
v1/v2 of this patch:

  WARNING: suspicious RCU usage in tipc_mon_delete
  net/tipc/monitor.c:108 suspicious rcu_dereference_check() usage!
  ...
  tipc_monitor_rcu_bh+0xf5/0x110  net/tipc/monitor.c:108
  get_self                        net/tipc/monitor.c:209
  tipc_mon_delete+0x10b/0x4d0     net/tipc/monitor.c:704

Drop get_self() entirely.  Each existing caller already has a valid
mon pointer from its initial RCU-correct load, and mon->self is the
result get_self() was returning.  Replace each "self = get_self(...)"
with "self = mon->self;".  This both removes the duplicate dereference
and fixes the lockdep splat.

synchronize_rcu() in tipc_mon_delete() is placed after
write_unlock_bh() and before timer_shutdown_sync() + kfree() so all
softirq readers that already observed the old pointer have completed
before the memory is freed.

Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
Cc: stable@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604301148.jfXKC9HF-lkp@intel.com/
Reported-by: syzbot ci <syzbot+ci779e8ed86620f383@syzkaller.appspotmail.com>
Closes: https://ci.syzbot.org/series/6267bc07-4172-4821-b3e5-dac381479d9d
Signed-off-by: SnailSploit | Kai Aizen <95986478+SnailSploit@users.noreply.github.com>
---
 net/tipc/core.h    |  2 +-
 net/tipc/monitor.c | 42 +++++++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/net/tipc/core.h b/net/tipc/core.h
index 9ce5f9ff6..cd582f7a2 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -109,7 +109,7 @@ struct tipc_net {
 	u32 num_links;
 
 	/* Neighbor monitoring list */
-	struct tipc_monitor *monitors[MAX_BEARERS];
+	struct tipc_monitor __rcu *monitors[MAX_BEARERS];
 	int mon_threshold;
 
 	/* Bearer list */
diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index a94b9b36a..0095a62ae 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -99,7 +99,14 @@ struct tipc_monitor {
 
 static struct tipc_monitor *tipc_monitor(struct net *net, int bearer_id)
 {
-	return tipc_net(net)->monitors[bearer_id];
+	return rcu_dereference_bh(tipc_net(net)->monitors[bearer_id]);
+}
+
+/* tipc_monitor_rtnl - dereference monitors[] from RTNL-held control path. */
+static struct tipc_monitor * __maybe_unused
+tipc_monitor_rtnl(struct net *net, int bearer_id)
+{
+	return rtnl_dereference(tipc_net(net)->monitors[bearer_id]);
 }
 
 const int tipc_max_domain_size = sizeof(struct tipc_mon_domain);
@@ -192,13 +199,6 @@ static struct tipc_peer *get_peer(struct tipc_monitor *mon, u32 addr)
 	return NULL;
 }
 
-static struct tipc_peer *get_self(struct net *net, int bearer_id)
-{
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
-
-	return mon->self;
-}
-
 static inline bool tipc_mon_is_active(struct net *net, struct tipc_monitor *mon)
 {
 	struct tipc_net *tn = tipc_net(net);
@@ -358,7 +358,7 @@ void tipc_mon_remove_peer(struct net *net, u32 addr, int bearer_id)
 	if (!mon)
 		return;
 
-	self = get_self(net, bearer_id);
+	self = mon->self;
 	write_lock_bh(&mon->lock);
 	peer = get_peer(mon, addr);
 	if (!peer)
@@ -422,9 +422,12 @@ static bool tipc_mon_add_peer(struct tipc_monitor *mon, u32 addr,
 void tipc_mon_peer_up(struct net *net, u32 addr, int bearer_id)
 {
 	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
-	struct tipc_peer *self = get_self(net, bearer_id);
+	struct tipc_peer *self;
 	struct tipc_peer *peer, *head;
 
+	if (!mon)
+		return;
+	self = mon->self;
 	write_lock_bh(&mon->lock);
 	peer = get_peer(mon, addr);
 	if (!peer && !tipc_mon_add_peer(mon, addr, &peer))
@@ -449,7 +452,7 @@ void tipc_mon_peer_down(struct net *net, u32 addr, int bearer_id)
 	if (!mon)
 		return;
 
-	self = get_self(net, bearer_id);
+	self = mon->self;
 	write_lock_bh(&mon->lock);
 	peer = get_peer(mon, addr);
 	if (!peer) {
@@ -651,7 +654,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
 	struct tipc_peer *self;
 	struct tipc_mon_domain *dom;
 
-	if (tn->monitors[bearer_id])
+	if (rtnl_dereference(tn->monitors[bearer_id]))
 		return 0;
 
 	mon = kzalloc_obj(*mon, GFP_ATOMIC);
@@ -663,7 +666,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
 		kfree(dom);
 		return -ENOMEM;
 	}
-	tn->monitors[bearer_id] = mon;
+	rcu_assign_pointer(tn->monitors[bearer_id], mon);
 	rwlock_init(&mon->lock);
 	mon->net = net;
 	mon->peer_cnt = 1;
@@ -682,16 +685,16 @@ int tipc_mon_create(struct net *net, int bearer_id)
 void tipc_mon_delete(struct net *net, int bearer_id)
 {
 	struct tipc_net *tn = tipc_net(net);
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
 	struct tipc_peer *self;
 	struct tipc_peer *peer, *tmp;
 
 	if (!mon)
 		return;
 
-	self = get_self(net, bearer_id);
+	self = mon->self;
+	RCU_INIT_POINTER(tn->monitors[bearer_id], NULL);
 	write_lock_bh(&mon->lock);
-	tn->monitors[bearer_id] = NULL;
 	list_for_each_entry_safe(peer, tmp, &self->list, list) {
 		list_del(&peer->list);
 		hlist_del(&peer->hash);
@@ -700,6 +703,7 @@ void tipc_mon_delete(struct net *net, int bearer_id)
 	}
 	mon->self = NULL;
 	write_unlock_bh(&mon->lock);
+	synchronize_rcu();
 	timer_shutdown_sync(&mon->timer);
 	kfree(self->domain);
 	kfree(self);
@@ -712,7 +716,7 @@ void tipc_mon_reinit_self(struct net *net)
 	int bearer_id;
 
 	for (bearer_id = 0; bearer_id < MAX_BEARERS; bearer_id++) {
-		mon = tipc_monitor(net, bearer_id);
+		mon = tipc_monitor_rtnl(net, bearer_id);
 		if (!mon)
 			continue;
 		write_lock_bh(&mon->lock);
@@ -798,7 +802,7 @@ static int __tipc_nl_add_monitor_peer(struct tipc_peer *peer,
 int tipc_nl_add_monitor_peer(struct net *net, struct tipc_nl_msg *msg,
 			     u32 bearer_id, u32 *prev_node)
 {
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
 	struct tipc_peer *peer;
 
 	if (!mon)
@@ -827,7 +831,7 @@ int tipc_nl_add_monitor_peer(struct net *net, struct tipc_nl_msg *msg,
 int __tipc_nl_add_monitor(struct net *net, struct tipc_nl_msg *msg,
 			  u32 bearer_id)
 {
-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
 	char bearer_name[TIPC_MAX_BEARER_NAME];
 	struct nlattr *attrs;
 	void *hdr;
-- 
2.43.0


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

* RE: [PATCH net v3] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
  2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
  2026-04-30 15:40   ` SnailSploit | Kai Aizen
@ 2026-05-02  3:35   ` Tung Quang Nguyen
  2026-05-02 16:42   ` Jakub Kicinski
  2026-05-05 13:01   ` Paolo Abeni
  3 siblings, 0 replies; 7+ messages in thread
From: Tung Quang Nguyen @ 2026-05-02  3:35 UTC (permalink / raw)
  To: SnailSploit | Kai Aizen
  Cc: stable@vger.kernel.org, jmaloy@redhat.com, ying.xue@windriver.com,
	kuba@kernel.org, pabeni@redhat.com,
	tipc-discussion@lists.sourceforge.net,
	tung.q.nguyen@dektech.com.au, lkp@intel.com,
	oe-kbuild-all@lists.linux.dev, syzkaller-bugs@googlegroups.com,
	netdev@vger.kernel.org, SnailSploit | Kai Aizen, syzbot ci

><syzbot+ci779e8ed86620f383@syzkaller.appspotmail.com>
>Subject: [PATCH net v3] tipc: fix UAF race in
>tipc_mon_peer_up/down/remove_peer vs bearer teardown
>
>From: "SnailSploit | Kai Aizen"
><95986478+SnailSploit@users.noreply.github.com>
>
>CVE-2025-40280 fixed tipc_mon_reinit_self() accessing monitors[] from a
>workqueue without RTNL.  That patch closed the workqueue path by adding
>rtnl_lock() around the call.
>
>However, three additional functions in the same subsystem access tipc_net-
>>monitors[] from softirq context with no RCU protection at all:
>
>  tipc_mon_peer_up()     - called from tipc_node_write_unlock()
>  tipc_mon_peer_down()   - called from tipc_node_write_unlock()
>  tipc_mon_remove_peer() - called from tipc_node_link_down()
>
>These are invoked from the packet receive path (tipc_rcv ->
>tipc_node_write_unlock / tipc_node_link_down) and hold only the per-node
>rwlock, not RTNL.
>
>Concurrently, bearer_disable() -- which always holds RTNL -- calls
>tipc_mon_delete(), which sets tn->monitors[bearer_id] = NULL and then
>kfree(mon) without an RCU grace period. A softirq reader can observe the
>non-NULL slot, take a reference, get preempted, and resume after
>kfree(mon) on another CPU, dereferencing freed memory.
>
>Convert monitors[] to __rcu, use rcu_assign_pointer() on creation,
>RCU_INIT_POINTER() + synchronize_rcu() on deletion before kfree(), and the
>appropriate dereference variant at each read site:
>
>  - tipc_monitor() returns rcu_dereference_bh(...) for softirq callers
>    (tipc_mon_peer_up/down/remove_peer/rcv/prep/get_state).
>  - tipc_monitor_rtnl() returns rtnl_dereference(...) for RTNL-held
>    callers (tipc_mon_delete via bearer_disable, tipc_mon_reinit_self
>    via tipc_net_finalize_work which wraps in rtnl_lock(), and the
>    netlink dump handlers tipc_nl_add_monitor_peer /
>    __tipc_nl_add_monitor).
>
>Also, get_self() was a thin wrapper over tipc_monitor() + ->self deref,
>duplicating the RCU-checked load that callers already perform on entry.
>With monitors[] becoming __rcu, get_self()'s use of tipc_monitor() generates a
>lockdep splat in tipc_mon_delete() (RTNL context) because the inner load is
>rcu_dereference_bh().  syzbot CI reported this on
>v1/v2 of this patch:
>
>  WARNING: suspicious RCU usage in tipc_mon_delete
>  net/tipc/monitor.c:108 suspicious rcu_dereference_check() usage!
>  ...
>  tipc_monitor_rcu_bh+0xf5/0x110  net/tipc/monitor.c:108
>  get_self                        net/tipc/monitor.c:209
>  tipc_mon_delete+0x10b/0x4d0     net/tipc/monitor.c:704
>
>Drop get_self() entirely.  Each existing caller already has a valid mon pointer
>from its initial RCU-correct load, and mon->self is the result get_self() was
>returning.  Replace each "self = get_self(...)"
>with "self = mon->self;".  This both removes the duplicate dereference and
>fixes the lockdep splat.
>
>synchronize_rcu() in tipc_mon_delete() is placed after
>write_unlock_bh() and before timer_shutdown_sync() + kfree() so all softirq
>readers that already observed the old pointer have completed before the
>memory is freed.
>
>Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
>Cc: stable@vger.kernel.org
>Reported-by: kernel test robot <lkp@intel.com>
>Closes: https://lore.kernel.org/oe-kbuild-all/202604301148.jfXKC9HF-
>lkp@intel.com/
>Reported-by: syzbot ci
><syzbot+ci779e8ed86620f383@syzkaller.appspotmail.com>
>Closes: https://ci.syzbot.org/series/6267bc07-4172-4821-b3e5-dac381479d9d
>Signed-off-by: SnailSploit | Kai Aizen
><95986478+SnailSploit@users.noreply.github.com>
>---
> net/tipc/core.h    |  2 +-
> net/tipc/monitor.c | 42 +++++++++++++++++++++++-------------------
> 2 files changed, 24 insertions(+), 20 deletions(-)
>
>diff --git a/net/tipc/core.h b/net/tipc/core.h index 9ce5f9ff6..cd582f7a2 100644
>--- a/net/tipc/core.h
>+++ b/net/tipc/core.h
>@@ -109,7 +109,7 @@ struct tipc_net {
> 	u32 num_links;
>
> 	/* Neighbor monitoring list */
>-	struct tipc_monitor *monitors[MAX_BEARERS];
>+	struct tipc_monitor __rcu *monitors[MAX_BEARERS];
> 	int mon_threshold;
>
> 	/* Bearer list */
>diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c index
>a94b9b36a..0095a62ae 100644
>--- a/net/tipc/monitor.c
>+++ b/net/tipc/monitor.c
>@@ -99,7 +99,14 @@ struct tipc_monitor {
>
> static struct tipc_monitor *tipc_monitor(struct net *net, int bearer_id)  {
>-	return tipc_net(net)->monitors[bearer_id];
>+	return rcu_dereference_bh(tipc_net(net)->monitors[bearer_id]);
Please use rcu_ dereference() because the read-side does not use RCU_bh markers.
>+}
>+
>+/* tipc_monitor_rtnl - dereference monitors[] from RTNL-held control
>+path. */ static struct tipc_monitor * __maybe_unused
>+tipc_monitor_rtnl(struct net *net, int bearer_id) {
Please use simple form like this for readability:
static struct tipc_monitor* tipc_monitor_rtnl(struct net *net, 
                                                                                  int bearer_id)
>+	return rtnl_dereference(tipc_net(net)->monitors[bearer_id]);
> }
>
> const int tipc_max_domain_size = sizeof(struct tipc_mon_domain); @@ -
>192,13 +199,6 @@ static struct tipc_peer *get_peer(struct tipc_monitor *mon,
>u32 addr)
> 	return NULL;
> }
>
>-static struct tipc_peer *get_self(struct net *net, int bearer_id) -{
>-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
>-
>-	return mon->self;
>-}
>-
> static inline bool tipc_mon_is_active(struct net *net, struct tipc_monitor
>*mon)  {
> 	struct tipc_net *tn = tipc_net(net);
>@@ -358,7 +358,7 @@ void tipc_mon_remove_peer(struct net *net, u32 addr,
>int bearer_id)
> 	if (!mon)
> 		return;
>
>-	self = get_self(net, bearer_id);
>+	self = mon->self;
> 	write_lock_bh(&mon->lock);
> 	peer = get_peer(mon, addr);
> 	if (!peer)
>@@ -422,9 +422,12 @@ static bool tipc_mon_add_peer(struct tipc_monitor
>*mon, u32 addr,  void tipc_mon_peer_up(struct net *net, u32 addr, int
>bearer_id)  {
> 	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
>-	struct tipc_peer *self = get_self(net, bearer_id);
>+	struct tipc_peer *self;
> 	struct tipc_peer *peer, *head;
>
>+	if (!mon)
>+		return;
>+	self = mon->self;
> 	write_lock_bh(&mon->lock);
> 	peer = get_peer(mon, addr);
> 	if (!peer && !tipc_mon_add_peer(mon, addr, &peer)) @@ -449,7
>+452,7 @@ void tipc_mon_peer_down(struct net *net, u32 addr, int
>bearer_id)
> 	if (!mon)
> 		return;
>
>-	self = get_self(net, bearer_id);
>+	self = mon->self;
> 	write_lock_bh(&mon->lock);
> 	peer = get_peer(mon, addr);
> 	if (!peer) {
>@@ -651,7 +654,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
> 	struct tipc_peer *self;
> 	struct tipc_mon_domain *dom;
>
>-	if (tn->monitors[bearer_id])
>+	if (rtnl_dereference(tn->monitors[bearer_id]))
> 		return 0;
>
> 	mon = kzalloc_obj(*mon, GFP_ATOMIC);
>@@ -663,7 +666,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
> 		kfree(dom);
> 		return -ENOMEM;
> 	}
>-	tn->monitors[bearer_id] = mon;
>+	rcu_assign_pointer(tn->monitors[bearer_id], mon);
> 	rwlock_init(&mon->lock);
> 	mon->net = net;
> 	mon->peer_cnt = 1;
>@@ -682,16 +685,16 @@ int tipc_mon_create(struct net *net, int bearer_id)
>void tipc_mon_delete(struct net *net, int bearer_id)  {
> 	struct tipc_net *tn = tipc_net(net);
>-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
>+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
> 	struct tipc_peer *self;
> 	struct tipc_peer *peer, *tmp;
>
> 	if (!mon)
> 		return;
>
>-	self = get_self(net, bearer_id);
>+	self = mon->self;
>+	RCU_INIT_POINTER(tn->monitors[bearer_id], NULL);
> 	write_lock_bh(&mon->lock);
>-	tn->monitors[bearer_id] = NULL;
> 	list_for_each_entry_safe(peer, tmp, &self->list, list) {
> 		list_del(&peer->list);
> 		hlist_del(&peer->hash);
>@@ -700,6 +703,7 @@ void tipc_mon_delete(struct net *net, int bearer_id)
> 	}
> 	mon->self = NULL;
> 	write_unlock_bh(&mon->lock);
>+	synchronize_rcu();
Please use kfree_rcu() instead.
> 	timer_shutdown_sync(&mon->timer);
> 	kfree(self->domain);
> 	kfree(self);
>@@ -712,7 +716,7 @@ void tipc_mon_reinit_self(struct net *net)
> 	int bearer_id;
>
> 	for (bearer_id = 0; bearer_id < MAX_BEARERS; bearer_id++) {
>-		mon = tipc_monitor(net, bearer_id);
>+		mon = tipc_monitor_rtnl(net, bearer_id);
> 		if (!mon)
> 			continue;
> 		write_lock_bh(&mon->lock);
>@@ -798,7 +802,7 @@ static int __tipc_nl_add_monitor_peer(struct tipc_peer
>*peer,  int tipc_nl_add_monitor_peer(struct net *net, struct tipc_nl_msg *msg,
> 			     u32 bearer_id, u32 *prev_node)
> {
>-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
>+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
> 	struct tipc_peer *peer;
>
> 	if (!mon)
>@@ -827,7 +831,7 @@ int tipc_nl_add_monitor_peer(struct net *net, struct
>tipc_nl_msg *msg,  int __tipc_nl_add_monitor(struct net *net, struct
>tipc_nl_msg *msg,
> 			  u32 bearer_id)
> {
>-	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
>+	struct tipc_monitor *mon = tipc_monitor_rtnl(net, bearer_id);
> 	char bearer_name[TIPC_MAX_BEARER_NAME];
> 	struct nlattr *attrs;
> 	void *hdr;
>--
>2.43.0
>


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

* Re: [PATCH net v3] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
  2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
  2026-04-30 15:40   ` SnailSploit | Kai Aizen
  2026-05-02  3:35   ` Tung Quang Nguyen
@ 2026-05-02 16:42   ` Jakub Kicinski
  2026-05-05 13:01   ` Paolo Abeni
  3 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2026-05-02 16:42 UTC (permalink / raw)
  To: SnailSploit | Kai Aizen
  Cc: netdev, stable, jmaloy, ying.xue, pabeni, tipc-discussion,
	tung.q.nguyen, lkp, oe-kbuild-all, syzkaller-bugs,
	SnailSploit | Kai Aizen, syzbot ci

On Thu, 30 Apr 2026 18:40:55 +0300 SnailSploit | Kai Aizen wrote:
> From: "SnailSploit | Kai Aizen" <95986478+SnailSploit@users.noreply.github.com>

We need a real email address. 
The correct way to include your company / sponsor name is in round
brackets, eg

	Kai Aizen (SnailSploit) <email...

please refer to the process docs for more info if necessary.
-- 
pw-bot: cr

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

* Re: [PATCH net v3] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
  2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
                     ` (2 preceding siblings ...)
  2026-05-02 16:42   ` Jakub Kicinski
@ 2026-05-05 13:01   ` Paolo Abeni
  3 siblings, 0 replies; 7+ messages in thread
From: Paolo Abeni @ 2026-05-05 13:01 UTC (permalink / raw)
  To: SnailSploit | Kai Aizen, netdev
  Cc: stable, jmaloy, ying.xue, kuba, tipc-discussion, tung.q.nguyen,
	lkp, oe-kbuild-all, syzkaller-bugs, SnailSploit | Kai Aizen,
	syzbot ci

On 4/30/26 5:26 PM, SnailSploit | Kai Aizen wrote:
> @@ -422,9 +422,12 @@ static bool tipc_mon_add_peer(struct tipc_monitor *mon, u32 addr,
>  void tipc_mon_peer_up(struct net *net, u32 addr, int bearer_id)
>  {
>  	struct tipc_monitor *mon = tipc_monitor(net, bearer_id);
> -	struct tipc_peer *self = get_self(net, bearer_id);
> +	struct tipc_peer *self;
>  	struct tipc_peer *peer, *head;

Minor nit: please respect the reverse christmas tree order above.

>  
> +	if (!mon)
> +		return;

Also an empty line here (and other similar places in the patch) will
make the code more readable.
> @@ -663,7 +666,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
>  		kfree(dom);
>  		return -ENOMEM;
>  	}
> -	tn->monitors[bearer_id] = mon;
> +	rcu_assign_pointer(tn->monitors[bearer_id], mon);
>  	rwlock_init(&mon->lock);
>  	mon->net = net;
>  	mon->peer_cnt = 1;

Sashiko says:

Does rcu_assign_pointer() publish the mon object before its lock
and fields are fully initialized?
Since rcu_assign_pointer() provides a release barrier, a concurrent
lockless RCU reader (like tipc_mon_peer_up()) could observe the new
mon pointer and attempt to acquire write_lock_bh(&mon->lock) before
rwlock_init(&mon->lock) has executed, or dereference a still-NULL
mon->self.
Should the publication step be moved to the absolute end of the
initialization sequence?

Note that sashiko has more remarks, even if they looks like pre-existing
issues to me.

/P


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

end of thread, other threads:[~2026-05-05 13:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-30 13:03 [PATCH net v2] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown Kai Zen
2026-03-31  3:46 ` Tung Quang Nguyen
2026-04-30 15:26 ` [PATCH net v3] " SnailSploit | Kai Aizen
2026-04-30 15:40   ` SnailSploit | Kai Aizen
2026-05-02  3:35   ` Tung Quang Nguyen
2026-05-02 16:42   ` Jakub Kicinski
2026-05-05 13:01   ` Paolo Abeni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox