Re: [PATCH v30 24/28] Audit: Add framework for auxiliary records

Paul Moore
2021-12-06T02:45:25Z

On Tue, Nov 23, 2021 at 9:10 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Add a list for auxiliary record data to the audit_buffer structure.
> Add the audit_stamp information to the audit_buffer as there's no
> guarantee that there will be an audit_context containing the stamp
> associated with the event. At audit_log_end() time create auxiliary
> records (none are currently defined) as have been added to the list.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> kernel/audit.c | 85 ++++++++++++++++++++++++++++++++++++++++++------
> kernel/audit.h | 1 +
> kernel/auditsc.c | 2 ++
> 3 files changed, 78 insertions(+), 10 deletions(-)

...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 069cd4c81a61..2b22498d3532 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2393,6 +2403,61 @@ void audit_log_end(struct audit_buffer *ab)
> wake_up_interruptible(&kauditd_wait);
> } else
> audit_log_lost("rate limit exceeded");
> +}
> +
> +/**
> + * audit_log_end - end one audit record
> + * @ab: the audit_buffer
> + *
> + * Let __audit_log_end() handle the message while the buffer housekeeping
> + * is done here.
> + * If there are other records that have been deferred for the event
> + * create them here.
> + */
> +void audit_log_end(struct audit_buffer *ab)
> +{
> + struct audit_context_entry *entry;
> + struct audit_context mcontext;
> + struct audit_context *mctx;
> + struct audit_buffer *mab;
> + struct list_head *l;
> + struct list_head *n;
> +
> + if (!ab)
> + return;
> +
> + __audit_log_end(ab);
> +
> + if (list_empty(&ab->aux_records)) {
> + audit_buffer_free(ab);
> + return;
> + }
> +
> + if (ab->ctx == NULL) {
> + mcontext.context = AUDIT_CTX_AUXRECORD;

More on this below, but I don't think we need, or want, the line above.

> diff --git a/kernel/audit.h b/kernel/audit.h
> index 56560846f3b0..f87da8e0a5a4 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -112,6 +112,7 @@ struct audit_context {
> AUDIT_CTX_UNUSED, /* audit_context is currently unused */
> AUDIT_CTX_SYSCALL, /* in use by syscall */
> AUDIT_CTX_URING, /* in use by io_uring */
> + AUDIT_CTX_AUXRECORD, /* in use for auxiliary records */
> } context;

We shouldn't need to do this here, and I wouldn't recommend this as a
solution even if we were running into problems in audit_log_exit().
The "context" field in the audit_context struct is to identify the
execution context of the task which is generating the audit record(s).

I'm trying to think of a case in this patchset where you would find
"audit_context->context == AUDIT_CTX_AUXRECORD" and I keep coming up
blank, are you certain you hit that case with the code you posted
here? If so, could you help me understand that situation?

--
paul moore