linux-kernel - Re: suspicious RCU usage in security/selinux/netnode.c

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]

Date:	Tue, 15 May 2012 10:52:07 -0400
From:	Eric Paris <eparis@...isplace.org>
To:	paulmck@...ux.vnet.ibm.com
Cc:	Dave Jones <davej@...hat.com>, sds@...ho.nsa.gov,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	paul@...l-moore.com
Subject: Re: suspicious RCU usage in security/selinux/netnode.c

On Tue, May 15, 2012 at 10:46 AM, Paul E. McKenney
<paulmck@...ux.vnet.ibm.com> wrote:
> On Tue, May 15, 2012 at 10:24:23AM -0400, Eric Paris wrote:
>> On Tue, May 15, 2012 at 1:16 AM, Paul E. McKenney
>> <paulmck@...ux.vnet.ibm.com> wrote:
>> > On Tue, May 15, 2012 at 12:41:45AM -0400, Dave Jones wrote:
>> >> I just triggered this on Linus' current tree.
>> >
>> > This is a bare:
>> >
>> >        rcu_dereference(sel_netnode_hash[idx].list.prev)
>> >
>> > which needs to be in an RCU read-side critical section.  Alternatively,
>> > the above should instead be something like:
>> >
>> >        rcu_dereference_check(sel_netnode_hash[idx].list.prev,
>> >                              lockdep_is_held(&sel_netnode_lock));
>>
>> Right, but that 'bare' dereference comes from
>> list_for_each_entry_rcu(), [from sel_netnode_sid_slow()] which I don't
>> see how to easily annotate with the lock.  Nor do I think it's within
>> my brain power (or my willingness to maintain such in the future) to
>> want to open code that logic.
>
> You lost me on this one.  The lockdep splat called out the
> rcu_dereference() above, not a list_for_each_entry_rcu().  Besides which,
> the list_for_each_entry_rcu() does not do the checking -- at the time,
> I was not willing to explode the API that much.

Ohhhh, ok.  I assumed we needed to annotate list_for_each_entry_rcu()
under the spinlock as well as the bare dereference in the insert code.
 Ok, should be very easy to fix, although the list running code is
still going to be un-annotated in any way.  Thanks

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/