[Crash-utility] feature to dump audit logs in vmcore
Dave Anderson
anderson at redhat.com
Tue Mar 14 10:42:51 UTC 2017
----- Original Message -----
>
>
> > -----Original Message-----
> > From: crash-utility-bounces at redhat.com
> > [mailto:crash-utility-bounces at redhat.com] On Behalf Of Dave Anderson
> > Sent: Tuesday, March 14, 2017 4:18 AM
> > To: Discussion list for crash utility usage, maintenance and development
> > <crash-utility at redhat.com>
> > Subject: Re: [Crash-utility] feature to dump audit logs in vmcore
> >
> >
> >
> > ----- Original Message -----
> >
> > > I've written the first version of the patch adding a feature to dump
> > > kernel
> > > audit logs as log -a.
> > > Could you review this patch?
> > >
> > > I made this patch on top of today's latest commit on github crash utility
> > > repository:
> > >
> > >
> > https://github.com/crash-utility/crash/commit/ed60e97e319a1cfc9e2779aa1baa
> > c305677393d8
> > >
> > > Thanks.
> > > HATAYAMA, Daisuke
> > >
> >
> > ----- Original Message -----
> >
> > > What the extension module does is so simple that it retrives audit
> > > logs from a queue. Looking back into kernel git logs, this design that
> > > audit logs are held in the queue, was introduced at the following
> > > patch to introduce kauditd kernel thread and have never changed until
> > > now:
> > >
> > > # git describe b7d1125817c9a46cc46f57db89d9c195e7af22f8
> > > v2.6.12-rc4-48-gb7d1125
> > >
> > > # git log -1 -p b7d1125817c9a46cc46f57db89d9c195e7af22f8
> > > commit b7d1125817c9a46cc46f57db89d9c195e7af22f8
> > > Author: David Woodhouse <dwmw2 shinybook infradead org>
> > > Date: Thu May 19 10:56:58 2005 +0100
> > >
> > > AUDIT: Send netlink messages from a separate kernel thread
> > > netlink_unicast() will attempt to reallocate and will free
> > > messages if the socket's rcvbuf limit is reached unless we give it an
> > > infinite timeout. So do that, from a kernel thread which is dedicated
> > > to spewing stuff up the netlink socket.
> > >
> > > Signed-off-by: David Woodhouse <dwmw2 infradead org>
> > >
> > > I confirmed the comamnd works well on fedora 4.8 kernel, RHEL7.3,
> > > RHEL6.8 and RHEL5.11; RHEL6.8 was tested both on x86_64 and x86_32.
> >
> > I tested this on ~250 sample vmcores I keep around, but absolutely none of them
> > had any audit data stored. So "log -a" either quietly did nothing, or in a handful
> > of cases, failed with "log: -a option not supported or applicable on this architecture
> > or kernel". Perhaps it would be helpful if you could also print a message if
> > there is no audit data stored in the kernel?
>
> Thanks for your reviewing.
>
> Sure, I add such message such as ''kernel audit buffer is empty''.
>
> >
> > Also, with respect to "have never changed until now", your patch does this:
>
> Sorry, 'never' was not sutable.
>
> I mean the relevant data structures such as sk_buff and the way handling them
> are not changed. A kind of queues and their handling have varied a little.
> So, I wanted to say it's not so much complicated to implement dumping feature
> of kernel audit logs.
>
> >
> > +static void
> > +__dump_audit(char *symname)
> > +{
> > + if (symbol_exists(symname)) {
> > + if (CRASHDEBUG(1))
> > + fprintf(fp, "# %s:\n", symname);
> > + dump_audit_skb_queue(symbol_value(symname));
> > + }
> > +}
> > +
> > +static void
> > +dump_audit(void)
> > +{
> > + if (!symbol_exists("audit_skb_queue"))
> > + option_not_supported('a');
> > +
> > + __dump_audit("audit_skb_hold_queue");
> > + __dump_audit("audit_skb_queue");
> > +}
> >
> > But in 4.10, the command fails due to this commit, which changed the names
> > of both of those two symbols above:
> >
> > commit af8b824f283de5acc9b9ae8dbb60e4adacff721b
> > Author: Paul Moore <paul at paul-moore.com>
> > Date: Tue Nov 29 16:53:24 2016 -0500
> >
> > audit: rename the queues and kauditd related functions
> >
> > The audit queue names can be shortened and the record sending
> > helpers associated with the kauditd task could be named better, do
> > these small cleanups now to make life easier once we start reworking
> > the queues and kauditd code.
> >
> > Signed-off-by: Paul Moore <paul at paul-moore.com>
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 801247a..6ac1df1 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -138,9 +138,9 @@
> > static int audit_freelist_count;
> > static LIST_HEAD(audit_freelist);
> >
> > -static struct sk_buff_head audit_skb_queue;
> > +static struct sk_buff_head audit_queue;
> > /* queue of skbs to send to auditd when/if it comes back */
> > -static struct sk_buff_head audit_skb_hold_queue;
> > +static struct sk_buff_head audit_hold_queue;
> > static struct task_struct *kauditd_task;
> > static DECLARE_WAIT_QUEUE_HEAD(kauditd_wait);
> > static DECLARE_WAIT_QUEUE_HEAD(audit_backlog_wait);
> > ...
> >
> > I don't know what else may have changed with the patch above, but the
> > "reworking the queuees and kauditd code" comment above sounds like the
> > whole infrastructure is due for some signficant changes soon. In any
> > case, it would be unfortunate to check in a new option that doesn't at
> > least work on the current stable upstream kernel.
>
> I've noticed this for the first time. I'll look into this.
>
> As the first look, it looks to me that this patch also doesn't change data
> structures and the way of handling them. It may be enough to adapt to
> changes of symbol names and addition of new kind of audit queues.
>
> >
> > With respect to the size_table changes, I understand why you might want
> > to store the structure member sizes of nlmsghdr.nlmsg_len, nlmsghdr.nlmsg_type
> > and and sk_buff_head.qlen, but it's overkill to bother storing the sizes of
> > sk_buff_head.next, sk_buff.data and sk_buff.next:
> >
> > @@ -2126,6 +2132,13 @@ struct size_table { /* stash of
> > commonly-used
> > sizes */
> > long task_struct_flags;
> > long timer_base;
> > long taint_flag;
> > + long nlmsghdr;
> > + long nlmsghdr_nlmsg_len;
> > + long nlmsghdr_nlmsg_type;
> > + long sk_buff_head_next;
> > + long sk_buff_head_qlen;
> > + long sk_buff_data;
> > + long sk_buff_next;
> > };
> >
> > Given that those 3 members are all pointer types, you could just use
> > "sizeof(void *)" for the readmem() calls, as is done throughout the
> > crash code. Note that there are not any MEMBER_SIZE_INIT() callers
> > for structure members that are known, guaranteed, pointer values.
>
> I'm doing this considering x86 build as x86_64 binary where pointer size
> differs, or is this unnecessary for such x86 build? Sorry, I didn't test
> x86 build as x86_64 binary.
With "make target=x86", crash is built with -m32 to create a 32-bit x86 binary
that runs on an x86_64 host, so kernel pointer sizes will always be the same
as the crash binary.
Thanks,
Dave
More information about the Crash-utility
mailing list