netdev - Re: [PATCH net] ipv6: sr: fix scheduling in RCU when creating seg6 lwtunnel state

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: <61d63ddf-eefa-3eca-25b4-51e2c9db2fc7@gmail.com>
Date:   Tue, 20 Mar 2018 10:20:15 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     David Lebrun <dav.lebrun@...il.com>,
        Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org
Cc:     David Lebrun <dlebrun@...gle.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>
Subject: Re: [PATCH net] ipv6: sr: fix scheduling in RCU when creating seg6
 lwtunnel state



On 03/20/2018 10:11 AM, David Lebrun wrote:
> On 20/03/18 15:07, Eric Dumazet wrote:
>> This is not the proper fix.
>>
>> Control path holds RTNL and can sleeep if needed.
>>
>> RCU should be avoided in lwtunnel_build_state()
>>
> 
> +Roopa
> 
> In lwtunnel_build_state(), the RCU protects the lwtunnel_encap_ops "ops" which is rcu-dereferenced. Moreover, the lwtunnel_state_alloc() function, which is used in all build_state functions, also uses GFP_ATOMIC, so this seemed a proper fix, or at least proper mitigation.
> 
> Do you suggest that the lwtunnel_encap_ops can be protected in a different way, not requiring RCU ?

Yes, GFP_ATOMIC might be an 'easy fix' for net tree,
but for the future, GFP_KERNEL allocations make more sense in control path.

Many scripts do not handle errors correctly and simply abort.
In case of failure, they might retry (busy poll) in the best scenario,
consuming cycles that might be needed to exit from pressure.

GFP_KERNEL allocations provide proper scheduling, they really should be the norm for control path.