Re: [PATCH 2/2] audit: Enable cacheing of cmdline in audit_context

On Mon, Nov 18, 2013 at 04:41:20PM -0800, William Roberts wrote: > Not enitrely sure if this is getting us any benefit, as in my > environment, these contexts are getting free'd immediately. > > Change-Id: Ia0d432bc4aba8588840f0dc0026a1e9483e5b485 > Signed-off-by: William Roberts <wroberts(a)tresys.com&gt; > --- > kernel/auditsc.c | 48 +++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 37 insertions(+), 11 deletions(-) > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 45fd3d0..4b30c5d 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -270,6 +270,7 @@ struct audit_context { > } mmap; > }; > int fds[2]; > + unsigned long cmdline; Would this be better declared to avoid all the casts?:

char *cmdline; > #if AUDIT_DEBUG > int put_count; > @@ -1044,6 +1045,14 @@ static inline void audit_free_aux(struct audit_context *context) > } > } > > +static inline void audit_cmdline_free(struct audit_context *ctx) > +{ > + if (!ctx->cmdline) > + return; > + free_page(ctx->cmdline); > + ctx->cmdline = 0; ctx->cmdline = NULL; > +} > + > static inline void audit_zero_context(struct audit_context *context, > enum audit_state state) > { > @@ -1118,6 +1127,7 @@ static inline void audit_free_context(struct audit_context *context) > audit_free_aux(context); > kfree(context->filterkey); > kfree(context->sockaddr); > + audit_cmdline_free(context); > kfree(context); > context = previous; > } while (context); > @@ -1154,35 +1164,51 @@ error_path: > > EXPORT_SYMBOL(audit_log_task_context); > > -static void audit_log_add_cmdline(struct audit_buffer *ab, > +static unsigned long audit_cmdline_get_page(struct audit_buffer *ab, static char* audit_cmdline_get_page(struct audit_buffer *ab, > struct task_struct *tsk) > { > int len; > unsigned long page; > - char *msg = "(null)"; > - > - audit_log_format(ab, " cmdline="); > > /* Get the process cmdline */ > page = __get_free_page(GFP_TEMPORARY); > if (!page) { > - audit_log_untrustedstring(ab, msg); > - return; > + return 0; return NULL; > } > len = proc_pid_cmdline(tsk, (char *)page); > if (len <= 0) { > free_page(page); > - audit_log_untrustedstring(ab, msg); > - return; > + return 0; return NULL; > } > /* > * Ensure NULL terminated! Application could > * could be using setproctitle(3). > */ > ((char *)page)[len-1] = '\0'; > - msg = (char *)page; > + > + /* TODO: Re-alloc to something smaller then a page here? */ > + return page; > +} > + > +static void audit_log_cmdline(struct audit_buffer *ab, struct task_struct *tsk, > + struct audit_context *context) > +{ > + char *msg = "(null)"; > + > + audit_log_format(ab, " cmdline="); > + > + /* Already cached */ > + if (context->cmdline) { > + msg = (char *)context->cmdline; msg = context->cmdline; > + goto out; > + } > + /* Not cached yet */ > + context->cmdline = audit_cmdline_get_page(ab, tsk); > + if (!context->cmdline) > + goto out; > + msg = (char *)context->cmdline; msg = context->cmdline; > +out: > audit_log_untrustedstring(ab, msg); > - free_page(page); > } > > static void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk) > @@ -1211,7 +1237,6 @@ static void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk > } > up_read(&mm->mmap_sem); > } > - audit_log_add_cmdline(ab, tsk); > audit_log_task_context(ab); > } > > @@ -1679,6 +1704,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts > > > audit_log_task_info(ab, tsk); > + audit_log_cmdline(ab, tsk, context); > audit_log_key(ab, context->filterkey); > audit_log_end(ab); > > -- > 1.7.9.5 > - RGB -- Richard Guy Briggs <rbriggs(a)redhat.com&gt; Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545