netdev - Re: [PATCH net-next] mld: add missing rtnl_lock() in do_ipv6

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:   Wed, 31 Mar 2021 01:13:25 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next] mld: add missing rtnl_lock() in
 do_ipv6_getsockopt()

On 3/31/21 1:08 AM, Eric Dumazet wrote:
 > On Tue, Mar 30, 2021 at 6:02 PM Taehee Yoo <ap420073@...il.com> wrote:
 >>
 >> On 3/31/21 12:40 AM, Eric Dumazet wrote:
 >>   > This seems a serious regression compared to old code (in net tree)
 >>   >
 >>   > Have you added RTNL requirement in all this code ?
 >>   >
 >>   > We would like to use RTNL only if strictly needed.
 >>
 >> Yes, I agree with you.
 >> This patchset actually relies on existed RTNL, which is
 >> setsockopt_needs_rtnl().
 >> And remained RTNL was replaced by mc_lock.
 >> So, this patchset actually doesn't add new RTNL except in this case.
 >>
 >> Fortunately, I think It can be replaced by RCU because,
 >> 1. ip6_mc_msfget() doesn't need the sleepable functions.
 >> 2. It is not the write critical section.
 >> So, RCU can be used instead of RTNL for ip6_mc_msfget().
 >> How do you think about it?
 >
 > Yes please, do not add RTNL here if we can avoid it.
 >
Okay, I will send a new patch.

 > Otherwise some applications will slow down the whole stack, even with
 > different containers/netns.
 >
 > (There is a single RTNL for the whole machine)
 >

Thanks a lot for the review!