Hi Darrel,
On Thu, Feb 16, 2006 at 02:09:04PM -0600, Darrel Goeddel wrote:
>>+int security_aurule_match(u32 ctxid, void *rule)
>>+{
>
><snip>
>
>>+ POLICY_RDLOCK;
>>+
>>+ if (aurule->au_seqno < latest_granting) {
>>+ context_destroy(&aurule->au_ctxt);
>>+ aurule->au_seqno = latest_granting;
>>+ aurule_init_context(aurule);
>>+ }
>
>
>Interesting approach; I was expecting to have the audit system
>register
>an AVC callback for reloads (similar to netif table) and initiate the
>re-processing of its audit rules at that time. And simply fail on
>filters with stale seqnos if there happened to be an interleaving
>with
>the policy reload. I suppose that this is more robust.
I was hoping you'd agree on this one. This idea seemed much simpler
to me and I think it avoids quite a bit of extra code for the rule
rebuilding.
I don't think it can be that simple. Historically, audit filter
operations have been read-only. Rcu is used because waiting on any
other kind of lock would be a bottleneck to the syscall path.
You are introducing a write operation to part of the filterlist while
we are only holding read locks (rcu_read_lock() in auditsc.c and
POLICY_RDLOCK here). This could conflict with other readers and
writers of this data.
One option is to introduce a field-specific lock. When audit rules
are configured such that the field applies to only a few syscalls,
then syscall processing isn't affected very much. However, we can't
dictate that audit rules are written in this way, so I doubt we can
make a case for this.
The other option requires audit to be aware of the update so it can
make a new copy of the rule and do a list_replace_rcu(). This
shouldn't happen during filtering though.
I'm of the opinion that syscall filtering should remain read-only.
Anything else isn't going to scale well.
Regards,
Amy