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

[Paul Moore <paul@paul-moore.com>]

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
www.paul-moore.com