lkml.org 
[lkml]   [2004]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: RCU issue with SELinux (Re: SELINUX performance issues)
Date
Hi Paul, thanks for your comments.

> > I modified the following points:
> > - We hold the lock for hash backet when avc_insert() and avc_ss_reset() are
> > called for safety.
> > - list_for_each_rcu() and list_entry() are replaced by list_for_entry().
>
> One subtlety here...
>
> The traversals that are protected by rcu_read_lock() (rather than an
> update-side spinlock) need to be list_for_each_entry_rcu() rather than
> list_for_each_entry(). The "_rcu()" is required in order to work
> reliably on Alpha, and has the added benefit of calling out exactly
> which traversals are RCU-protected.
>
> Update-side code remains list_for_each_entry().

It was a simple misconception.
I fixed them in the take3-patch.

> > - avc_node_dual structure which contains two avc_node objects is defined.
> > It allows to do avc_update_node() without kmalloc() or any locks.
>
> What happens when you have two consecutive updates to the same object?
> Don't you have to defer the second update until a grace period has
> elapsed since the first update in order to avoid confusing readers that
> are still accessing the original version?

I didn't imagine such a situation. Indeed, such thing may happen.

> One way to do this would be to set a "don't-touch-me" bit that is
> cleared by an RCU callback. An update to an element with the
> "don't-touch-me" bit set would block until the bit clears. There
> are probably better ways...

I think we can't apply this approach for the implementation
of avc_update_node(), because execution context isn't permitted to block.

I changed my opinion and implementation of avc_update_node().
If kmalloc() returns NULL in avc_update_node(), it returns -ENOMEM.

But this effect of changing the prototype is limited, because only
avc_has_perm_noaudit() and avc_update_cache() call avc_update_node().

Even if avc_update_node() return -ENOMEM to avc_has_perm_noaudit(),
avc_has_perm_noaudit() can ignore it, because the purpose is only
to control the audit-log floods.
This adverse effect is only that audit-logs are printed twice.

Nobody calls avc_update_cache(), which is only defined.

Some other trivial fixes are as follows:
- All list_for_each_entry() were replaced by list_for_each_entry_rcu().
- All spin_lock()/spin_unlock() were replaced by spin_lock_irqsave()
/spin_unlock_restore().
- In avc_node_insert(), if an entry with the same ssid/tsid/tclass as new
one exists, the older entry is replaced by the new one.

Thank you for the opinion as a specialist of RCU!
--------
Kai Gai <kaigai@ak.jp.nec.com>

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

\
 
 \ /
  Last update: 2005-03-22 14:05    [W:0.402 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site