netdev - Re: [PATCH net-next 2/2] net/ipv6: Fix missing rcu dereferences on from

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 24 Apr 2018 08:54:03 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/2] net/ipv6: Fix missing rcu dereferences on
 from



On 04/23/2018 11:32 AM, David Ahern wrote:
> kbuild test robot reported 2 uses of rt->from not properly accessed
> using rcu_dereference:
> 1. add rcu_dereference_protected to rt6_remove_exception_rt and make
>    sure it is always called with rcu lock held.
> 
> 2. change rt6_do_redirect to take a reference on 'from' when accessed
>    the first time so it can be used the sceond time outside of the lock
> 
> Fixes: a68886a69180 ("net/ipv6: Make from in rt6_info rcu protected")
> Reported-by: kbuild test robot <lkp@...el.com>
> Signed-off-by: David Ahern <dsahern@...il.com>
> ---
>  net/ipv6/route.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 354a5b8d016f..ac3e51631c65 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1541,11 +1541,13 @@ static struct rt6_info *rt6_find_cached_rt(struct fib6_info *rt,
>  static int rt6_remove_exception_rt(struct rt6_info *rt)
>  {
>  	struct rt6_exception_bucket *bucket;
> -	struct fib6_info *from = rt->from;
>  	struct in6_addr *src_key = NULL;
>  	struct rt6_exception *rt6_ex;
> +	struct fib6_info *from;
>  	int err;
>  
> +	from = rcu_dereference_protected(rt->from,
> +					 lockdep_is_held(&rt6_exception_lock));

This does not make any sense.

We lock rt6_exception_lock a bit later in this function (line 1558)

If we really were holding rt6_exception_lock here we would dead lock.

>  	if (!from ||
>  	    !(rt->rt6i_flags & RTF_CACHE))
>  		return -EINVAL;
> @@ -2223,6 +2225,7 @@ static void ip6_link_failure(struct sk_buff *skb)