Re: [PATCH 2/2] security/smack implement logging V2

From: Casey Schaufler
Date: Sun Apr 05 2009 - 22:20:27 EST


Etienne Basset wrote:
>> It looks like the only difference between these are their non-logging
>> versions is the logging. I say go ahead and add the auditdata parameter
>> to smk_access and smk_curacc and allow for the case where it is NULL.
>>
>>
> i would prefer keeping a logging and a non-logging variant
> maybe rename smk_access & smk_curacc to smk_access_nolog & smk_curacc_nolog
> Or you really prefer that we pass a NULL when we want the non-logging?
> I guess its a matter of taste, so i let you decide :)
>

I dislike functions that do nothing but set up another function.


>> It would be nice if audit data was only manipulated if audit is
>> installed, but I don't like the idea of ifdeffing everything
>> very much either. How about a static inline in smack.h that is
>> ifdeffed for audit? smack_audit_init? There would need to be one
>> for each field, too.
>>
>>
> why not, but instead of a "initialiser" for each field i'll prefer a macro
> To save some stack we could also conditionnaly define the "common_audit_data"
>
> something like :
>
> #ifdef CONFIG_AUDIT
> #define DEFINE_SMACK_AUDIT_DATA(ad) struct common_audit_data ad
> void smack_audit_init(..) {
> COMMON_AUDIT_DATA_INIT(...)
> }
> #define SMACK_AUDIT_DATA_SET_FIELD(_ad, _field, _value) (_ad._field = _value)
> ...
> #else
> #define DEFINE_SMACK_AUDIT_DATA(ad)
> void smack_audit_init(..)
> {
> }
> #define SMACK_AUDIT_DATA_SET_FIELD(_ad, _field, _value)
> #endif
>

I'm not convinced. #defines are good for assigning names to constants,
but I saw the original Bourne shell code, where defines were used to
make the code look like ALGOL68, so you have to overcome the fear that
was instilled in me when I was young.



>> Now this is tricky. You'll get an audit record for single-label
>> hosts, but not for those that use CIPSO. The former is good, the
>> latter is bad.
>>
>>
> gargll you're right
>

I love those french idioms.

Actually, I don't think there's much point in worrying about it.
The audit you do here is correct, and until you can get more subject
information over the wire there isn't much point doing it on the
receiving end.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/