[RFC] [PATCH]

[Amy Griffis]

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