[PATCH 2/2] audit string fields interface + consumer

[Serge E. Hallyn]

Quoting Timothy R. Chavez (tinytim at us.ibm.com):
> On Tue, 2006-01-17 at 17:19 -0500, Amy Griffis wrote:
> > Here is an update that incorporates changes based on Tim's feedback:
> >     - sanity check for path with trailing /
> >     - call path_release
> >     - use audit_compare_watch
> > 
> > and fixes panics due to the assumption of the existence of
> > rule->watch.
> >
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index f3b2a00..cc979e9 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -52,6 +52,12 @@ enum audit_state {
> >  };
> >  
> >  /* Rule lists */
> > +struct audit_watch {
> > +	char			*path; /* watch insertion path */
> > +	struct list_head	mlist; /* entry in master_watchlist */
> > +	struct list_head	rules; /* associated rules */
> > +};
> > +
> >  struct audit_field {
> >  	u32			type;
> >  	u32			val;
> > @@ -67,6 +73,8 @@ struct audit_krule {
> >  	u32			buflen; /* for data alloc on list rules */
> >  	u32			field_count;
> >  	struct audit_field	fields[AUDIT_MAX_FIELDS];
> > +	struct audit_watch	*watch; /* associated watch */
> > +	struct list_head	rlist; /* entry in audit_watch.rules list */
> >  };
> 
> This may not really be that important, but if you switch to hlist_head
> you have a 4-byte savings, which is something...
> 
> AUDIT_MAX_FIELDS defaults to 64, sizeof(audit_field) is 12-bytes...
> 768-bytes... 788-bytes per audit_krule?

Agree.

> Not sure if this qualifies as stack abuse though, which is why I say it
> might not be that important.  I'm still a little concerned about
> AUDIT_MAX_FIELDS though... How much of that stack space is actually
> being used on average?

Why is it done that way?  I assume ->fields would only need to be allocated
or expanded when rules are being added?  So is there any reason not to
make ->fields and ->values dynamically allocated as needed?

More inanely, in

>+static int audit_to_watch(char *path, struct audit_krule *krule, int fidx)
>+{
>+	int err;
...
>+	err = -EINVAL;
>+	if (path[0] != '/' || path[f->val-1] == '/' ||
>+	    krule->listnr != AUDIT_FILTER_EXIT ||
>+	    f->op & ~AUDIT_EQUAL)
>+		return err;
...
>+	err = -ENOMEM;
>+	watch = kmalloc(sizeof(*watch), GFP_KERNEL);
>+	if (!watch)
>+		return err;
...
>+
>+	return 0;
>+}

There's really no point using err... :)

Anyway, still trying to get a full picture with all the patches to
analyze locking...

-serge