[PATCH 1/7] audit: implement generic feature setting and retrieving
William Roberts
bill.c.roberts at gmail.com
Fri May 24 20:56:38 UTC 2013
A couple other comments I missed before:
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32
*/
Why &31 --> if someone adds more than 5 things, they will git dropped out...
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & (~(__u32)0))) /* mask
for __u32 */
something like this perhaps?
Also
static char *audit_feature_names[2] = { ...
Drop the number, I had to inc this when I added mine...
Bill
On Fri, May 24, 2013 at 1:41 PM, William Roberts
<bill.c.roberts at gmail.com>wrote:
> 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
>
>
--
Respectfully,
William C Roberts
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/linux-audit/attachments/20130524/85a94993/attachment.htm>
More information about the Linux-audit
mailing list