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

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(a)redhat.com&gt; 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(a)redhat.com&gt; > > 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(a)redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit > -- Respectfully, William C Roberts