[PATCH] new audit rule interface

[Amy Griffis]

On Tue, Jan 03, 2006 at 06:23:48PM -0500, Steve Grubb wrote:
> > > > +???????char *str;
> > > > +
> > > > +???????if (!*bufp || (len > *remaining))
> > > > +???????????????return ERR_PTR(-EINVAL);
> > >
> > > len should always be <= PATH_MAX*2. That must be checked for as well or
> > > there could be wrapping.
> >
> > I'm confused about the significance of the value PATH_MAX*2 and how
> > that relates here. Could you clarify?
> 
> In order to make sure people aren't playing games by abusing the audit system, 
> I was thinking that you could do a check that the length isn't too big also. 
> If you have a u32 for len, and someone passes -1, then you have a large 
> positive number. To detect these problems, you have to check for a legal 
> upper bounds. In this case, the longest string should be PATH_MAX. I guess 
> the "*2" only applies to actual audit records rather than the file path for 
> rules. 

I understand the situation you're trying to address, but PATH_MAX may
not make sense as a bound for other string fields.  Wouldn't checking
the specified string field length against the actual size of the
provided buffer suffice?

> > > > -/* Check to see if two rules are identical. ?It is called from
> > > > - * audit_add_rule during AUDIT_ADD and
> > > > - * audit_del_rule during AUDIT_DEL. */
> > > > -static int audit_compare_rule(struct audit_rule *a, struct audit_rule
> > > > *b) +/* Translate struct audit_rule_xprt to kernel's rule
> > > > respresentation. */ +static struct audit_entry
> > > > *audit_xprt_to_entry(struct audit_rule_xprt *xprt,
> > > > +??????????????????????????????????????? ? ? ? int dlen) ?{
> > > > +???????int err = 0;
> > > > +???????struct audit_entry *entry;
> > > > +???????void *bufp;
> > > > +???????/* int remaining = dlen - sizeof(struct audit_rule_xprt); */
> > > > ????????int i;
> > > > ?
> > > > -???????if (a->flags != b->flags)
> > > > -???????????????return 1;
> > > > +???????entry = audit_to_entry_common((struct audit_rule *)xprt);
> > > > +???????if (IS_ERR(entry))
> > > > +???????????????goto exit_nofree;
> > > > +
> > > > +???????bufp = xprt->buf;
> > >
> > > What uses bufp?
> >
> > Nothing right now, but it should be used by a consumer adding code to
> > the switch statement.
> 
> OK, I thought the patch was complete. Nevermind.

Well, it is intended to be complete as an interface.

> 
> >
> > > > +/* Pack a filter field's string representation into data block. */
> > > > +static inline int audit_pack_string(void **bufp, char *str)
> > >
> > > What calls this?
> >
> > This should be called by a consumer from the switch in
> > audit_krule_to_xprt().
> 
> I really need to see the consumer to finish evaluating the use of the 
> interface.

Makes sense.  I'll post a consumer patch with the next iteration.