[PATCH net v3] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown

[SnailSploit | Kai Aizen]

From: "SnailSploit | Kai Aizen" <95986478+S...@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: sta...@vger.kernel.org
Reported-by: kernel test robot <l...@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604301148...@intel.com/
Reported-by: syzbot ci <syzbot+ci779...@syzkaller.appspotmail.com>
Closes: https://ci.syzbot.org/series/6267bc07-4172-4821-b3e5-dac381479d9d
Signed-off-by: SnailSploit | Kai Aizen <95986478+S...@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

[SnailSploit | Kai Aizen]

[Jakub Kicinski]

On Thu, 30 Apr 2026 18:40:55 +0300 SnailSploit | Kai Aizen wrote:
> From: "SnailSploit | Kai Aizen" <95986478+S...@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

[Tung Quang Nguyen]

><syzbot+ci779...@syzkaller.appspotmail.com>
>Subject: [PATCH net v3] tipc: fix UAF race in
>tipc_mon_peer_up/down/remove_peer vs bearer teardown

>Closes: https://lore.kernel.org/oe-kbuild-all/202604301148.jfXKC9HF-
>l...@intel.com/

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)

Please use kfree_rcu() instead.

[Paolo Abeni]

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