netdev - Re: [PATCH] NETLABEL: Fix an RCU warning

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: <201003251010.45128.paul.moore@hp.com>
Date:	Thu, 25 Mar 2010 10:10:44 -0400
From:	Paul Moore <paul.moore@...com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	David Howells <dhowells@...hat.com>, netdev@...r.kernel.org,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH] NETLABEL: Fix an RCU warning

On Thursday 25 March 2010 09:32:20 am Eric Dumazet wrote:
> Le jeudi 25 mars 2010 à 11:37 +0000, David Howells a écrit :
> > Eric Dumazet <eric.dumazet@...il.com> wrote:
> > > Sorry this is not the right fix.
> > > 
> > > Fix is to change the dereference check to take into account the lock
> > > owned here.
> > 
> > Then the comments on netlbl_unlhsh_hash(), netlbl_unlhsh_search_iface(),
> > netlbl_unlhsh_search_iface_def() and netlbl_unlhsh_add_iface() are all
> > wrong,
> > 
> > for all of them say:
> > 	* The caller is responsible for calling the rcu_read_[un]lock()
> > 	* functions.
> > 
> > Furthermore, netlabel_unlhsh_add() and netlabel_unlhsh_remove() _do_ wrap
> > the calls to those functions in rcu_read_lock'd sections.
> 
> Current code is probably fine.
> Comments are not up to date (as many other comments BTW)
> 
> Only the dereference check is bad, as it assumes the rcu_read lock is
> held.
> 
> Its not the case, we own a spinlock.
> 
> You suggest adding a surrounding rcu lock, but this surrounding lock
> adds overhead on normal kernels, to correct checker warnings only.

The rcu_dereference call, and the outdated comments for that matter, are 
likely artifacts from an early patch.  When the code was originally being 
developed I had assumed that you _always_ needed to hold the RCU read lock 
when doing RCU operations, regardless of if you were already holding the 
associated spinlock.  Thankfully, Paul McKenney was able to review the code 
and helped me to understand RCU a bit better, unfortunately, it looks like I 
still missed a few spots :/

Give me a day or two to go through the comments again and do some sanity 
checks and I'll send out a patch.  However, if you've already got something 
that is tested and ready to go don't let me stand in your way.

-- 
paul moore
linux @ hp
--
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