netdev - Re: [PATCH][net-next] ipv6: sanitize RCU usage on fib6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Feb 2019 15:08:38 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Li RongQing <lirongqing@...du.com>, netdev@...r.kernel.org
Subject: Re: [PATCH][net-next] ipv6: sanitize RCU usage on fib6_next



On 02/22/2019 01:44 AM, Li RongQing wrote:
> using rcu_assign_pointer when setting, which has a memory
> barrier to ensure the initialization is seen first.
> 
> using rcu_dereference when dereference this pointer
> 
> Signed-off-by: Zhang Yu <zhangyu31@...du.com>
> Signed-off-by: Li RongQing <lirongqing@...du.com>
> ---
>  net/ipv6/ip6_fib.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 6613d8dbb0e5..b73d40d68178 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1143,7 +1143,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
>  
>  		atomic_inc(&rt->fib6_ref);
>  		rcu_assign_pointer(rt->fib6_node, fn);
> -		rt->fib6_next = iter->fib6_next;
> +		rcu_assign_pointer(rt->fib6_next, iter->fib6_next);

We do not need a barrier here, the object is still private.



>  		rcu_assign_pointer(*ins, rt);
>  		if (!info->skip_notify)
>  			inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
> @@ -1761,7 +1761,9 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
>  	RT6_TRACE("fib6_del_route\n");
>  
>  	/* Unlink it */
> -	*rtp = rt->fib6_next;
> +	*rtp = rcu_dereference_protected(rt->fib6_next,
> +				    lockdep_is_held(&rt->fib6_table->tb6_lock));

This change will likely add a sparse bug.

> +
>  	rt->fib6_node = NULL;
>  	net->ipv6.rt6_stats->fib_rt_entries--;
>  	net->ipv6.rt6_stats->fib_discarded_routes++;
> 

Really I do not believe these changes are needed.

barriers should be added blindly.