Re: [RFC PATCH ghak90 (was ghak32) V3 04/10] audit: add support for non-syscall auxiliary records

On Tue, Jul 24, 2018 at 3:40 PM Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > On 2018-07-20 18:14, Paul Moore wrote: > > On Wed, Jun 6, 2018 at 1:01 PM Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > > > Standalone audit records have the timestamp and serial number generated > > > on the fly and as such are unique, making them standalone. This new > > > function audit_alloc_local() generates a local audit context that will > > > be used only for a standalone record and its auxiliary record(s). The > > > context is discarded immediately after the local associated records are > > > produced. > > > > > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > > > --- > > > include/linux/audit.h | 8 ++++++++ > > > kernel/auditsc.c | 25 +++++++++++++++++++++++-- > > > 2 files changed, 31 insertions(+), 2 deletions(-) ... > > > + struct audit_context *context; > > > + > > > + if (!audit_ever_enabled) > > > + return NULL; /* Return if not auditing. */ > > > + > > > + context = audit_alloc_context(AUDIT_RECORD_CONTEXT); > > > + if (!context) > > > + return NULL; > > > + context->serial = audit_serial(); > > > + context->ctime = current_kernel_time64(); > > > + context->in_syscall = 1; > > > > Setting in_syscall is both interesting and a bit troubling, if for no > > other reason than I expect most (all?) callers to be in an interrupt > > context when audit_alloc_local() is called. Setting in_syscall would > > appear to be conceptually in this case. Can you help explain why this > > is the right thing to do, or necessary to ensure things are handled > > correctly? > > I'll admit this is cheating a bit, but seemed harmless. It is needed so > that auditsc_get_stamp() from audit_get_stamp() from audit_log_start() > doesn't bail on me without giving me its already assigned time and > serial values rather than generating a new one. I did look to see if > there were any other undesireable side effects and found none, so I'm > tmepted to rename the ->in_syscall to something a bit more helpful. I > could add a new audit_context structure member to make > auditsc_get_stamp() co-operative, but this seems wasteful and > unnecessary. That's what I suspected. Let's look into renaming the "in_syscall" field, it borderline confusing now, and hijacking it for something which is very obviously not "in syscall" is A Very Bad Thing.

paul moore