netdev - Re: [Patch net-next] fib: move fib_rules_cleanup

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]

Message-ID: <551A101C.8080005@redhat.com>
Date:	Mon, 30 Mar 2015 20:10:20 -0700
From:	Alexander Duyck <alexander.h.duyck@...hat.com>
To:	Cong Wang <cwang@...pensource.com>
CC:	Thomas Graf <tgraf@...g.ch>, Cong Wang <xiyou.wangcong@...il.com>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [Patch net-next] fib: move fib_rules_cleanup_ops() under rtnl
 lock


On 03/30/2015 05:12 PM, Cong Wang wrote:
> On Mon, Mar 30, 2015 at 5:02 PM, Alexander Duyck
> <alexander.h.duyck@...hat.com> wrote:
>> On 03/30/2015 04:47 PM, Cong Wang wrote:
>>> As long as we agree rtnl lock should be taken, you already take my point
>>> here ($subject says so).
>>
>> Yes, I agree lock can be held.  For fib4 it was already holding the RTNL
>> lock when it made that call.  You can update the other users of
>> fib_rules_unregister so that they call it with the RTNL lock held as well.
>>
>>> It is just API change to move rtnl_lock up to caller or whatever
>>> appropriate.
>>
>> Right, so like I said for fib4 this is resolved.  That just leaves ipmr,
>> ip6mr, fib6, and dn_rules that need to be updated so that they correctly
>> handle the RTNL locking in their exit/cleanup paths. Since you already have
>> some related patches out for these I will let you take them otherwise I
>> might try to go through and clean them up next week.
>>
> Ok, then we are finally on the same page. We need two patches:
>
> 1) move unregister under rtnl lock (as what this patch intended to do)

Yes I think the only disagreement was on how to do it.  Your original 
patch placed it in fib_rules_cleanup_ops, and the preference of myself 
and Thomas was to hold the lock before you even call 
fib_rules_unregister.  So the IPv4 code is fine with the patch I 
submitted, it is the other callers of fib_rules_unregister which must be 
updated.

> 2) remove the unnecessary rules_mod_lock
>
> Thanks.

Please define "unnecessary" as we have had a bit of back and forth on 
how our views can differ there.  As far as I know it still has to be 
held for the fib_rule_ops list manipulation, specifically the call to 
list_del_rcu.  However, it doesn't need to be held when we call 
fib_rules_cleanup_ops.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html