From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f66.google.com ([209.85.215.66]:36896 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750989AbeDTQVf (ORCPT ); Fri, 20 Apr 2018 12:21:35 -0400 Received: by mail-lf0-f66.google.com with SMTP id b23-v6so5977863lfg.4 for ; Fri, 20 Apr 2018 09:21:35 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180420012346.udnga5pfdjoazcfc@madcap2.tricolour.ca> References: <20180420012346.udnga5pfdjoazcfc@madcap2.tricolour.ca> From: Paul Moore Date: Fri, 20 Apr 2018 12:21:33 -0400 Message-ID: Subject: Re: [RFC PATCH ghak32 V2 06/13] audit: add support for non-syscall auxiliary records To: Richard Guy Briggs Cc: cgroups@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, Linux-Audit Mailing List , linux-fsdevel@vger.kernel.org, LKML , netdev@vger.kernel.org, ebiederm@xmission.com, luto@kernel.org, jlayton@redhat.com, carlos@redhat.com, dhowells@redhat.com, viro@zeniv.linux.org.uk, simo@redhat.com, Eric Paris , serge@hallyn.com Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Apr 19, 2018 at 9:23 PM, Richard Guy Briggs wrote: > On 2018-04-18 20:39, Paul Moore wrote: >> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs 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 >> > --- >> > include/linux/audit.h | 8 ++++++++ >> > kernel/auditsc.c | 20 +++++++++++++++++++- >> > 2 files changed, 27 insertions(+), 1 deletion(-) >> > >> > diff --git a/include/linux/audit.h b/include/linux/audit.h >> > index ed16bb6..c0b83cb 100644 >> > --- a/include/linux/audit.h >> > +++ b/include/linux/audit.h >> > @@ -227,7 +227,9 @@ static inline int audit_log_container_info(struct audit_context *context, >> > /* These are defined in auditsc.c */ >> > /* Public API */ >> > extern int audit_alloc(struct task_struct *task); >> > +extern struct audit_context *audit_alloc_local(void); >> > extern void __audit_free(struct task_struct *task); >> > +extern void audit_free_context(struct audit_context *context); >> > extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1, >> > unsigned long a2, unsigned long a3); >> > extern void __audit_syscall_exit(int ret_success, long ret_value); >> > @@ -472,6 +474,12 @@ static inline int audit_alloc(struct task_struct *task) >> > { >> > return 0; >> > } >> > +static inline struct audit_context *audit_alloc_local(void) >> > +{ >> > + return NULL; >> > +} >> > +static inline void audit_free_context(struct audit_context *context) >> > +{ } >> > static inline void audit_free(struct task_struct *task) >> > { } >> > static inline void audit_syscall_entry(int major, unsigned long a0, >> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> > index 2932ef1..7103d23 100644 >> > --- a/kernel/auditsc.c >> > +++ b/kernel/auditsc.c >> > @@ -959,8 +959,26 @@ int audit_alloc(struct task_struct *tsk) >> > return 0; >> > } >> > >> > -static inline void audit_free_context(struct audit_context *context) >> > +struct audit_context *audit_alloc_local(void) >> > { >> > + 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; >> > + return context; >> > +} >> > + >> > +inline void audit_free_context(struct audit_context *context) >> > +{ >> > + if (!context) >> > + return; >> > audit_free_names(context); >> > unroll_tree_refs(context, NULL, 0); >> > free_tree_refs(context); >> >> I'm reserving the option to comment on this idea further as I make my >> way through the patchset, but audit_free_context() definitely >> shouldn't be declared as an inline function. > > Ok, I think I follow. When it wasn't exported, inline was fine, but now > that it has been exported, it should no longer be inlined ... Pretty much. Based on a few comments I've seen by compiler folks over the years, my current thinking is that we shouldn't worry about explicit inlining static functions in C files (header files are a different story). The basic idea being that the compiler almost always does a better job than us stupid developers. > ... or should use > an intermediate function name to export so that local uses of it can > remain inline. Possibly, but my guess is that the compiler could (will?) do that by itself for code that lives in the same file. -- paul moore www.paul-moore.com