On Thu, Apr 19, 2018 at 9:23 PM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
On 2018-04-18 20:39, Paul Moore wrote:
> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb(a)redhat.com> 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>
> > ---
> > 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