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] [day] [month] [year] [list]

Date:	Tue, 31 Mar 2015 10:56:33 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	Alexander Duyck <alexander.h.duyck@...hat.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 Tue, Mar 31, 2015 at 10:30 AM, Alexander Duyck
<alexander.h.duyck@...hat.com> wrote:
>
> On 03/31/2015 09:47 AM, Cong Wang wrote:
>>
>> On Mon, Mar 30, 2015 at 8:10 PM, Alexander Duyck
>> <alexander.h.duyck@...hat.com> wrote:
>>>
>>> On 03/30/2015 05:12 PM, Cong Wang wrote:
>>>>
>>>> 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.
>>>
>> Look at where rules_mod_lock are held: either when the net is initialized
>> or when unregistering, neither of them really needs this per netns lock:
>> new netns is not ready to expose;
>> concurrent unregistering is prevented by upper layer locking,
>> readers (lookup_rules_ops) hold RCU but we already should hold rtnl lock
>> (after patch of course).
>
>
> I would have almost agreed with you, however I noticed that decnet doesn't
> seem to follow the same rules as the rest of the callers to
> fib_rules_register.  It will simply call into module_init -> decnet_init ->
> dn_fib_init -> dn_fib_rules_injt -> fib_rules_register(&init_net).  It
> probably just needs to be rewritten to use
> register_pernet_subsys/unregister_pernet_subsys, or correctly use the
> net_mutex, and then you could probably go head and tear out the
> rules_mod_lock since they would all be covered under the net_mutex.
>

Good catch, or it can simply hold rtnl lock in module init path.
Anyway, it is for net-next.
--
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