Re: [RFC PATCH ghak32 V2 06/13] audit: add support for non-syscall auxiliary records

2018-04-20 Thread Paul Moore 
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

Re: [RFC PATCH ghak32 V2 06/13] audit: add support for non-syscall auxiliary records

2018-04-19 Thread Richard Guy Briggs 
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, or should use
an intermediate function name to export so that local uses of it can
remain inline.

> paul moore

- RGB

--
Richard Guy Briggs 
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH ghak32 V2 06/13] audit: add support for non-syscall auxiliary records

2018-04-18 Thread Paul Moore 
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.

-- 
paul moore
www.paul-moore.com

[RFC PATCH ghak32 V2 06/13] audit: add support for non-syscall auxiliary records

2018-03-16 Thread Richard Guy Briggs 
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);
-- 
1.8.3.1