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

[Paolo Abeni <pabeni@redhat.com>]

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