lkml.org 
[lkml]   [2019]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH ghak105 V2] audit: remove audit_context when CONFIG_ AUDIT and not AUDITSYSCALL
On Tue, Jan 29, 2019 at 6:18 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2019-01-29 18:07, Paul Moore wrote:
> > On Mon, Jan 28, 2019 at 1:33 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Remove audit_context from struct task_struct and struct audit_buffer
> > > when CONFIG_AUDIT is enabled but CONFIG_AUDITSYSCALL is not.
> > >
> > > Also, audit_log_name() (and supporting inode and fcaps functions) should
> > > have been put back in auditsc.c when soft and hard link logging was
> > > normalized since it is only used by syscall auditing.
> > >
> > > See github issue https://github.com/linux-audit/audit-kernel/issues/105
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > > Changelog:
> > > v2:
> > > - resolve merge conflicts from rebase on upstreamed ghak103 patch
> > > - wrap task_struct audit_context in CONFIG_AUDITSYSCALL
> > >
> > > include/linux/sched.h | 4 +-
> > > kernel/audit.c | 157 +++-----------------------------------------------
> > > kernel/audit.h | 9 ---
> > > kernel/auditsc.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 161 insertions(+), 159 deletions(-)
> >
> > ...
> >
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index 3f3f1888cac7..15e41603fd34 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -205,7 +205,9 @@ struct audit_net {
> > > * use simultaneously. */
> > > struct audit_buffer {
> > > struct sk_buff *skb; /* formatted skb ready to send */
> > > +#ifdef CONFIG_AUDITSYSCALL
> > > struct audit_context *ctx; /* NULL or associated context */
> > > +#endif
> > > gfp_t gfp_mask;
> > > };
> > >
> > > @@ -1696,7 +1698,9 @@ static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx,
> > > if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0))
> > > goto err;
> > >
> > > +#ifdef CONFIG_AUDITSYSCALL
> > > ab->ctx = ctx;
> > > +#endif
> >
> > I vaguely remember reading/hearing something in the past about
> > kmem_cache_alloc() not returning a zero'd out buffer in all cases, can
> > you say for certain that "ab" in this case is always going to be
> > zero'd out? This is an honest question.
>
> Ok, then maybe we should be using kmem_cache_zalloc() instead of
> kmem_cache_alloc() in audit_buffer_alloc()? (as I've done in
> the last patch of ghak81/first patch of ghak90)
>
> If this is too much overhead, then we can initialize ctx = NULL;

We don't need zalloc() since we're setting all the fields, although
more on this below ...

> > If we can't guarantee that "ab" is zero'd out, we should manually set
> > ab->ctx to NULL when !CONFIG_AUDITSYSCALL.
>
> But ctx isn't part of struct audit_buffer when !CONFIG_AUDITSYSCALL. It
> is #ifdef-ed out. What am I missing?

You're not, I am. I saw the obvious bit where you removed it from the
task_struct, but completely glossed over the bit where you also
removed it from the audit_buffer struct. My mistake.

Once the audit container ID stuff lands we are going to need to have
the audit_context pointer in the audit_buffer regardless of the
CONFIG_AUDITSYSCALL setting, right? Assuming the answer is yes, I
think I'd just assume leave the pointer in the audit_buffer (setting
it to NULL when !CONFIG_AUDITSYSCALL) so we don't have to have those
#ifdef's in the middle of the functions (I generally like to avoid
those if possible). I think it's still worth making the changes to
task_struct, as that is the right thing to do and doesn't have the
same level of impact.

--
paul moore
www.paul-moore.com

\
 
 \ /
  Last update: 2019-01-30 00:27    [W:0.055 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site