Re: [PATCH] improved xfrm_audit_log() patch
From: Joy Latten <latten(a)austin.ibm.com>
Date: Wed, 22 Aug 2007 20:29:17 -0500
On Wed, 2007-08-22 at 12:51 -0700, David Miller wrote:
> From: David Miller <davem(a)davemloft.net>
> Date: Tue, 21 Aug 2007 00:24:05 -0700 (PDT)
>
> > Looks good, applied to net-2.6.24, thanks Joy.
>
> Something is still buggered up in this patch, you can't add this local
> "audit_info" variable unconditionally to these functions, and
> alternatively you also can't add a bunch of ifdefs to xfrm_user.c to
> cover it up either.
>
I wonder if I am subconsciously trying to break a record or
something! My apologies as time is valuable.
I mean to get this right. My rationale for using audit_info was to
reduce amount of arguments to xfrm_audit_log(). However, I now like
it better when I just called xfrm_audit_log(NETLINK_CB(skb).loginuid,
NETLINK_CB(skb).sid, ...). User determines where/how to get loginuid and
secid and nothing happens when AUDIT not configured. But would make
xfrm_audit_log() have 7 arguments instead of 6.
My alternative is to remove xfrm_get_auditinfo() out of the
#ifdef CONFIG_AUDITSYSCALL and always fill in audit_info
regardless if AUDIT is configured or not. Less calls to
xfrm_audit_log() but perhaps unnecessary info when AUDIT
not configured.
Would first solution be acceptable?
I don't like either of these ideas, sorry.
I would suggest, at this point, to make purpose built situation
specific interfaces that pass specific objects (the ones being
operated upon) to the audit layer.
Let the audit layer pick out the bits it actually wants in the
format it likes.
For example, if we're creating a template, pass the policy and
the templace to the audit layer via a function called:
xfrm_audit_template_add()
or something like that. That function only needs two arguments.
All of these call sites will rarely need more than 2 or 3 arguments in
any given situation, and the on-stack audit thing will be gone too.
This is the suggestion I made to you over a month ago, but you choose
to do the on-stack thing.
You must make this cost absolutely nothing when it is either
not configured, and have next to no cost when not enabled at
run time. And it is very doable.