Re: [PATCH net 2/2] nexthop: avoid unlocked f6i_list walk in nh_rt_cache_flush

[Ido Schimmel]

On Wed, Jul 22, 2026 at 12:29:51AM +0000, Xiang Mei (Microsoft) wrote:
> nh_rt_cache_flush() walks nh->f6i_list during an RTNL-serialized nexthop
> replace without holding nh->lock, racing the unlocked IPv6 route
> add/delete that mutate the list under nh->lock and free fib6_info
> entries (nh_rt_cache_flush() is inlined into rtm_new_nexthop()):
> 
>   BUG: KASAN: slab-use-after-free in nh_rt_cache_flush (net/ipv4/nexthop.c:2243)
>   Read of size 8 at addr ffff888012953e18 by task exploit/146
>    nh_rt_cache_flush (net/ipv4/nexthop.c:2243)
>    replace_nexthop (net/ipv4/nexthop.c:2610)
>    rtm_new_nexthop (net/ipv4/nexthop.c:3323)
>    rtnetlink_rcv_msg (net/core/rtnetlink.c:7076)
> 
> Unlike the other f6i_list walks, this one bumps each route's sernum via
> fib6_update_sernum_upto_root(), which needs tb6_lock; taking nh->lock
> around it would invert the established tb6_lock -> nh->lock order and
> deadlock. As the only purpose is to invalidate cached dsts, bump the
> IPv6 sernum for the whole netns with rt_genid_bump_ipv6() instead,
> mirroring the rt_cache_flush() already done for IPv4 just above.
> 
> Fixes: 081efd18326e ("ipv6: Protect nh->f6i_list with spinlock and flag.")
> Reported-by: AutonomousCodeSecurity@xxxxxxxxxxxxx
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@xxxxxxx>
> ---
>  net/ipv4/nexthop.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index c1d3b7d7100d..1e9ccac6bf0f 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -2240,18 +2240,18 @@ static void remove_one_nexthop(struct net *net, struct nexthop *nh,
>  static void nh_rt_cache_flush(struct net *net, struct nexthop *nh,
>  			      struct nexthop *replaced_nh)
>  {
> -	struct fib6_info *f6i;
>  	struct nh_group *nhg;
> +	bool have_f6i;
>  	int i;
>  
>  	if (!list_empty(&nh->fi_list))
>  		rt_cache_flush(net);
>  
> -	list_for_each_entry(f6i, &nh->f6i_list, nh_list) {
> -		spin_lock_bh(&f6i->fib6_table->tb6_lock);
> -		fib6_update_sernum_upto_root(net, f6i);
> -		spin_unlock_bh(&f6i->fib6_table->tb6_lock);
> -	}
> +	spin_lock_bh(&nh->lock);
> +	have_f6i = !list_empty(&nh->f6i_list);
> +	spin_unlock_bh(&nh->lock);
> +	if (have_f6i)
> +		rt_genid_bump_ipv6(net);

Can you look into converting this list to RCU in net-next so that we
wouldn't need rt_genid_bump_ipv6() and instead restore
fib6_update_sernum_upto_root()?

Note that rt_genid_bump_ipv6() is much more expensive than
rt_cache_flush().

Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>

>  
>  	/* if an IPv6 group was replaced, we have to release all old
>  	 * dsts to make sure all refcounts are released
> -- 
> 2.43.0
>