netdev - Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}

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: <20160331054301.GA57227@ast-mbp.thefacebook.com>
Date:	Wed, 30 Mar 2016 22:43:03 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Michal Kubecek <mkubecek@...e.cz>
Cc:	Daniel Borkmann <daniel@...earbox.net>, davem@...emloft.net,
	sasha.levin@...cle.com, jslaby@...e.cz, eric.dumazet@...il.com,
	mst@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net] tun, bpf: fix suspicious RCU usage in
 tun_{attach,detach}_filter

On Thu, Mar 31, 2016 at 07:22:32AM +0200, Michal Kubecek wrote:
> On Wed, Mar 30, 2016 at 10:08:10PM -0700, Alexei Starovoitov wrote:
> > On Thu, Mar 31, 2016 at 07:01:15AM +0200, Michal Kubecek wrote:
> > > On Wed, Mar 30, 2016 at 06:18:42PM -0700, Alexei Starovoitov wrote:
> > > > 
> > > > kinda heavy patch to shut up lockdep.
> > > > Can we do
> > > > old_fp = rcu_dereference_protected(sk->sk_filter,
> > > >                                 sock_owned_by_user(sk) || lockdep_rtnl_is_held());
> > > > and it always be correct?
> > > > I think right now tun is the only such user, but if it's correct
> > > > for tun, it's correct for future users too. If not correct then
> > > > not correct for tun either.
> > > > Or I'm missing something?
> > > 
> > > Already discussed here:
> > > 
> > >   http://thread.gmane.org/gmane.linux.kernel/2158069/focus=405853
> > 
> > I saw that. My point above was challenging 'less accurate' part.
> > 
> Daniel's point was that lockdep_rtnl_is_held() does not mean "we hold
> RTNL" but "someone holds RTNL" so that some other task holding RTNL at
> the moment could make the check happy even when called by someone
> supposed to own the socket.

Of course... and that is the case for all rtnl_dereference() calls...
yet we're not paranoid about it.