[PATCH 1/7] audit: implement generic feature setting and retrieving

[William Roberts]

Looking through the patch, these are my thoughts:

I like that my "splitlog" patch shrunk a lot, way easier. I like that. It
also proves something like this is the correct direction.

Shouldn't audit_set_feature() check the version number, granted it doesn't
mater now, but shouldn't their be a:

 if(uaf->version <= AUDIT_FEATURE_SUPPORTED_VERSION)


This branchy code could be:
if (new_feature)
af.features |= feature;
else
af.features &= ~feature;

changed to:
af.features = (af.features & ~feature) | feature

Ie just do a clear bit than or in what you want.

Otherwise LGTM

Bill





On Fri, May 24, 2013 at 9:28 AM, Eric Paris <eparis at redhat.com> wrote:

> On Fri, 2013-05-24 at 12:11 -0400, Eric Paris wrote:
> > The audit_status structure was not designed with extensibility in mind.
> > Define a new AUDIT_SET_FEATURE message type which takes a new structure
> > of bits where things can be enabled/disabled/locked one at a time.  This
> > structure should be able to grow in the future while maintaining forward
> > and backward compatibility (based loosly on the ideas from capabilities
> > and prctl)
> >
> > This does not actually add any features, but is just infrastructure to
> > allow new on/off types of audit system features.
> >
> > Signed-off-by: Eric Paris <eparis at redhat.com>
>
> Attached you will find the test program I used to check that things were
> working correctly.  It should give an idea to Steve how we can program
> the features support in userspace.  I believe it fits very nicely to
> have a new syntax in audit.rules to set (and lock if needed/wanted)
> these features.
>
> netlink.c is just some helper code I stole from the audit tree to get
> some functions which weren't exposed externally.  The only part really
> interesting is test.c.
>
> You will also need the include/uapi/linux/audit.h file from this patch
> to build test.c
>
> -Eric
>
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>



-- 
Respectfully,

William C Roberts
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/linux-audit/attachments/20130524/2cdd465d/attachment.htm>