| Message ID | ee2a945fb09a939b3c214f45e49dab6a770d83e6.1521179281.git.rgb@redhat.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | audit: implement container id | expand |
On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@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@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.
On 2018-04-18 20:39, Paul Moore wrote: > On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@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@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, or should use an intermediate function name to export so that local uses of it can remain inline. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> 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
On Thu, Apr 19, 2018 at 9:23 PM, Richard Guy Briggs <rgb@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@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@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.
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);
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@redhat.com> --- include/linux/audit.h | 8 ++++++++ kernel/auditsc.c | 20 +++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)