| Message ID | 21e6c4e1ac179c8dcf35803e603899ccfc69300a.1593198710.git.rgb@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On Sat, Jun 27, 2020 at 9:22 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. We've had some good discussions on the list about why we can't reuse the "in_syscall" field and need to add a "local" field, I think it would be good to address that here in the commit description. > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > Acked-by: Serge Hallyn <serge@hallyn.com> > Acked-by: Neil Horman <nhorman@tuxdriver.com> > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > include/linux/audit.h | 8 ++++++++ > kernel/audit.h | 1 + > kernel/auditsc.c | 33 ++++++++++++++++++++++++++++----- > 3 files changed, 37 insertions(+), 5 deletions(-) ... > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 9e79645e5c0e..935eb3d2cde9 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -908,11 +908,13 @@ static inline void audit_free_aux(struct audit_context *context) > } > } > > -static inline struct audit_context *audit_alloc_context(enum audit_state state) > +static inline struct audit_context *audit_alloc_context(enum audit_state state, > + gfp_t gfpflags) > { > struct audit_context *context; > > - context = kzalloc(sizeof(*context), GFP_KERNEL); > + /* We can be called in atomic context via audit_tg() */ At this point I think it's clear we need a respin so I'm not going to preface all of my nitpick comments as such, although this definitely would qualify ... I don't believe audit_tg() doesn't exist yet, likely coming later in this patchset, so please remove this comment as it doesn't make sense in this context. To be frank, don't re-add the comment later in the patchset either. Comments like these tend to be fragile and don't really add any great insight. The audit_tg() function can, and most likely will, be modified at some point in the future such that the comment above no longer applies, and there is a reasonable chance that when it does the above comment will not be updated. Further, anyone modifying the audit_alloc_context() is going to look at the callers (rather they *should* look at the callers) and will notice the no-sleep requirements. > @@ -960,8 +963,27 @@ int audit_alloc_syscall(struct task_struct *tsk) > return 0; > } > > -static inline void audit_free_context(struct audit_context *context) > +struct audit_context *audit_alloc_local(gfp_t gfpflags) > { > + struct audit_context *context = NULL; > + > + context = audit_alloc_context(AUDIT_RECORD_CONTEXT, gfpflags); > + if (!context) { > + audit_log_lost("out of memory in audit_alloc_local"); > + goto out; You might as well just return NULL here, no need to jump and then return NULL. > + } > + context->serial = audit_serial(); > + ktime_get_coarse_real_ts64(&context->ctime); > + context->local = true; > +out: > + return context; > +} > +EXPORT_SYMBOL(audit_alloc_local); -- paul moore www.paul-moore.com
diff --git a/include/linux/audit.h b/include/linux/audit.h index 89cf7c66abe6..15d0defc5193 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -330,6 +330,8 @@ static inline int audit_signal_info(int sig, struct task_struct *t) /* These are defined in auditsc.c */ /* Public API */ +extern struct audit_context *audit_alloc_local(gfp_t gfpflags); +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); @@ -592,6 +594,12 @@ static inline void audit_log_nfcfg(const char *name, u8 af, extern int audit_n_rules; extern int audit_signals; #else /* CONFIG_AUDITSYSCALL */ +static inline struct audit_context *audit_alloc_local(gfp_t gfpflags) +{ + return NULL; +} +static inline void audit_free_context(struct audit_context *context) +{ } static inline void audit_syscall_entry(int major, unsigned long a0, unsigned long a1, unsigned long a2, unsigned long a3) diff --git a/kernel/audit.h b/kernel/audit.h index 0c9446f8d52c..a7f88d76163f 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -98,6 +98,7 @@ struct audit_proctitle { struct audit_context { int dummy; /* must be the first element */ int in_syscall; /* 1 if task is in a syscall */ + bool local; /* local context needed */ enum audit_state state, current_state; unsigned int serial; /* serial number for record */ int major; /* syscall number */ diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 9e79645e5c0e..935eb3d2cde9 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -908,11 +908,13 @@ static inline void audit_free_aux(struct audit_context *context) } } -static inline struct audit_context *audit_alloc_context(enum audit_state state) +static inline struct audit_context *audit_alloc_context(enum audit_state state, + gfp_t gfpflags) { struct audit_context *context; - context = kzalloc(sizeof(*context), GFP_KERNEL); + /* We can be called in atomic context via audit_tg() */ + context = kzalloc(sizeof(*context), gfpflags); if (!context) return NULL; context->state = state; @@ -948,7 +950,8 @@ int audit_alloc_syscall(struct task_struct *tsk) return 0; } - if (!(context = audit_alloc_context(state))) { + context = audit_alloc_context(state, GFP_KERNEL); + if (!context) { kfree(key); audit_log_lost("out of memory in audit_alloc_syscall"); return -ENOMEM; @@ -960,8 +963,27 @@ int audit_alloc_syscall(struct task_struct *tsk) return 0; } -static inline void audit_free_context(struct audit_context *context) +struct audit_context *audit_alloc_local(gfp_t gfpflags) { + struct audit_context *context = NULL; + + context = audit_alloc_context(AUDIT_RECORD_CONTEXT, gfpflags); + if (!context) { + audit_log_lost("out of memory in audit_alloc_local"); + goto out; + } + context->serial = audit_serial(); + ktime_get_coarse_real_ts64(&context->ctime); + context->local = true; +out: + return context; +} +EXPORT_SYMBOL(audit_alloc_local); + +void audit_free_context(struct audit_context *context) +{ + if (!context) + return; audit_free_module(context); audit_free_names(context); unroll_tree_refs(context, NULL, 0); @@ -972,6 +994,7 @@ static inline void audit_free_context(struct audit_context *context) audit_proctitle_free(context); kfree(context); } +EXPORT_SYMBOL(audit_free_context); static int audit_log_pid_context(struct audit_context *context, pid_t pid, kuid_t auid, kuid_t uid, unsigned int sessionid, @@ -2204,7 +2227,7 @@ void __audit_inode_child(struct inode *parent, int auditsc_get_stamp(struct audit_context *ctx, struct timespec64 *t, unsigned int *serial) { - if (!ctx->in_syscall) + if (!ctx->in_syscall && !ctx->local) return 0; if (!ctx->serial) ctx->serial = audit_serial();