netdev - Re: Latest net-next from GIT panic

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:   Thu, 21 Sep 2017 11:06:18 +0200
From:   Paweł Staszewski <pstaszewski@...are.pl>
To:     Eric Dumazet <eric.dumazet@...il.com>, Wei Wang <weiwan@...gle.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: Latest net-next from GIT panic



W dniu 2017-09-21 o 03:17, Eric Dumazet pisze:
> On Wed, 2017-09-20 at 18:09 -0700, Wei Wang wrote:
>>> Thanks very much Pawel for the feedback.
>>>
>>> I was looking into the code (specifically IPv4 part) and found that in
>>> free_fib_info_rcu(), we call free_nh_exceptions() without holding the
>>> fnhe_lock. I am wondering if that could cause some race condition on
>>> fnhe->fnhe_rth_input/output so a double call on dst_dev_put() on the
>>> same dst could be happening.
>>>
>>> But as we call free_fib_info_rcu() only after the grace period, and
>>> the lookup code which could potentially modify
>>> fnhe->fnhe_rth_input/output all holds rcu_read_lock(), it seems
>>> fine...
>>>
>> Hi Pawel,
>>
>> Could you try the following debug patch on top of net-next branch and
>> reproduce the issue check if there are warning msg showing?
>>
>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 93568bd0a352..82aff41c6f63 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -271,7 +271,7 @@ static inline void dst_use_noref(struct dst_entry
>> *dst, unsigned long time)
>>   static inline struct dst_entry *dst_clone(struct dst_entry *dst)
>>   {
>>          if (dst)
>> -               atomic_inc(&dst->__refcnt);
>> +               dst_hold(dst);
>>          return dst;
>>   }
>>
>> Thanks.
>> Wei
>>
>
> Yes, we believe skb_dst_force() and skb_dst_force_safe() should be
> unified  (to the 'safe' version)
>
> We no longer have gc to protect from 0 -> 1 transition of dst refcount.
>
>
>
>

After adding patch from Wei
https://bugzilla.kernel.org/show_bug.cgi?id=197005#c14