On 2018-07-24 17:57, Paul Moore wrote:

On Tue, Jul 24, 2018 at 3:40 PM Richard Guy Briggs [off-list ref] wrote:

quoted

On 2018-07-20 18:14, Paul Moore wrote:

quoted

On Wed, Jun 6, 2018 at 1:01 PM Richard Guy Briggs [off-list ref] wrote:

quoted

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 <redacted>
---
 include/linux/audit.h |  8 ++++++++
 kernel/auditsc.c      | 25 +++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

...

quoted

quoted

quoted

+       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.

Ok, looking more carefully, I'm not going to touch in_syscall, since it
does more than I remember discovering when investigating why the
existing stamp wasn't being used.  I don't want to change the existing
behaviour.  I'll somewhat reluctantly grow the context struct and add a
"local" boolean to it so that auditsc_get_stamp knows to use the
existing stamp in both the in_syscall and local cases.

paul moore

- RGB