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

[Eric Dumazet <eric.dumazet@gmail.com>]

On 04/24/2018 08:54 AM, Eric Dumazet wrote:
> 
> 
> 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@intel.com>
>> Signed-off-by: David Ahern <dsahern@gmail.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.

I will send this fix :

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ac3e51631c659b5c5c8a93c17011cb7f3ad266e2..432c4bcc1111085671f32987e4673e47898085a3 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1546,8 +1546,7 @@ static int rt6_remove_exception_rt(struct rt6_info *rt)
        struct fib6_info *from;
        int err;
 
-       from = rcu_dereference_protected(rt->from,
-                                        lockdep_is_held(&rt6_exception_lock));
+       from = rcu_dereference(rt->from);
        if (!from ||
            !(rt->rt6i_flags & RTF_CACHE))
                return -EINVAL;