netdev - Re: VXLAN RCU error

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]

Message-ID: <20170224114012.0755d8c6@cakuba.netronome.com>
Date:   Fri, 24 Feb 2017 11:40:12 -0800
From:   Jakub Kicinski <kubakici@...pl>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        Pravin B Shelar <pshelar@....org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: VXLAN RCU error

On Fri, 24 Feb 2017 11:30:05 -0800, Cong Wang wrote:
> On Thu, Feb 23, 2017 at 12:20 AM, Jakub Kicinski <kubakici@...pl> wrote:
> >
> > Ugh.  Looks like this may not work even if it makes the splat go away.
> > synchronize_net() doesn't seem to wait for the _bh() flavor of RCU, so
> > we need to add syncronize_rcu_bh() call before freeing the socket or do
> > a normal rcu_read_lock()/unlock() on the fast path.  Any RCU experts
> > want to comment? :)  
> 
> But rcu_read_lock_bh() is enforced by upper layer, __dev_queue_xmit(),
> I am afraid you can't change it.
> 
> Does changing these kfree() to kfree_rcu() work too since there is no
> kfree_rcu_bh()?

If I read the code correctly the VXLAN/GENEVE does standard RCU
dereferencing (i.e. non-_bh()) but as you point out __dev_queue_xmit()
takes rcu_read_lock_bh() and not rcu_read_lock() so we either have to
make VXLAN code take the correct lock itself or make VXLAN
reconfiguration code do synchronize_rcu_bh() as well as
synchronize_net() (most likely with synchronize_rcu_mult() but then
we will have to open code expediting under rtnl...)  

I was waiting for some internal reviews but let me just post the code I
have now implementing the first option.