Re: [PATCH ghak90 V9 07/13] audit: add support for non-syscall auxiliary records
On Sat, Jun 27, 2020 at 9:22 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.
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(a)redhat.com>
Acked-by: Serge Hallyn <serge(a)hallyn.com>
Acked-by: Neil Horman <nhorman(a)tuxdriver.com>
Reviewed-by: Ondrej Mosnacek <omosnace(a)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