From: Casey Schaufler <casey@schaufler-ca.com>
To: Etienne Basset <etienne.basset@numericable.fr>
Cc: LSM <linux-security-module@vger.kernel.org>,
Eric Paris <eparis@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-audit@redhat.com, Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH 2/2] security/smack implement logging V2
Date: Sun, 05 Apr 2009 19:20:11 -0700 [thread overview]
Message-ID: <49D966DB.9070009@schaufler-ca.com> (raw)
In-Reply-To: <49D917AB.8000400@numericable.fr>
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.
next prev parent reply other threads:[~2009-04-06 2:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-05 8:47 [PATCH 2/2] security/smack implement logging V2 Etienne Basset
2009-04-05 17:49 ` Casey Schaufler
2009-04-05 20:42 ` Etienne Basset
2009-04-06 2:20 ` Casey Schaufler [this message]
2009-04-06 6:17 ` Etienne Basset
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49D966DB.9070009@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=eparis@redhat.com \
--cc=etienne.basset@numericable.fr \
--cc=linux-audit@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox