lkml.org 
[lkml]   [2004]   [Aug]   [24]   [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 Stephen, Thanks for your comments.

> I'm not overly familiar with RCU myself, but the comments in list.h for
> list_add_rcu suggest that you still need to hold a lock to avoid racing
> with another list_add_rcu or list_del_rcu call on the same list. But
> avc_insert is calling list_add_rcu without holding any lock; can't it
> race with another avc_insert on the same hash bucket? Do I just
> misunderstand, or is this unsafe? Thanks for clarifying.

You are right. Indeed, the lock for hash bucket is also necessary
when avc_insert() is called. I fixed them.

> I think we can likely eliminate the mutation of the node in the
> !selinux_enforcing case in avc_has_perm_noaudit, i.e. eliminate the
> entire else clause and just fall through with rc still 0. Adding the
> requested permissions to the node was simply to avoid flooding denials
> in permissive mode on the same permission check, but this can be
> addressed separately using the audit ratelimit mechanism.

I have another opinion.
This simple mechanism against the flood of audit log is necessary,
because it prevents the depletion of the system log buffer and denied log
all over the console when we are debugging the security policy in permissive mode.
So, I improved the avc_update_node() function and avc_node data structure.
It does not need kmalloc() when avc_update_node().

This approach is good for durability and compatibility of original implementation,
but double area of avc_nodes is needed for updating without kmalloc().
This approach can apply to any kinds of updating of avc_entry.
This idea is pretty complexer, though.

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().
- avc_node_dual structure which contains two avc_node objects is defined.
It allows to do avc_update_node() without kmalloc() or any locks.

Any comments please. Thanks.
--------
Kai Gai <kaigai@ak.jp.nec.com>[unhandled content-type:application/octet-stream][unhandled content-type:application/octet-stream]
\
 
 \ /
  Last update: 2005-03-22 14:05    [from the cache]
©2003-2011 Jasper Spaans