Re: [Crash-utility] feature to dump audit logs in vmcore

----- Original Message ----- ----- Original Message ----- 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? Also, with respect to "have never changed until now", your patch does this: +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(a)paul-moore.com&gt; 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(a)paul-moore.com&gt; 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. 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. Also, can you please display the new offset_table[] and size_table[] members in dump_offset_table() for use by "help -o"? Thanks, Dave