On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <r...@redhat.com> wrote:
>
> AUDIT_TIME_* events are generated when there are syscall rules present that
> are
> not related to time keeping. This will produce noisy log entries that could
> flood the logs and hide events we really care about.
>
> Rather than immediately produce the AUDIT_TIME_* records, store the data and
> log it at syscall exit time respecting the filter rules.
>
> Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919
>
> Signed-off-by: Richard Guy Briggs <r...@redhat.com>
> ---
> Note: This is a quick and dirty proof-of-concept. If this approach of
> storing the values in the audit_context for later filtering is
> acceptable I'll clean up the patch (re-name functions) and re-submit.
>
> kernel/audit.h | 6 ++++++
> kernel/auditsc.c | 29 +++++++++++++++++++++++++----
> 2 files changed, 31 insertions(+), 4 deletions(-)
Reviewing this now with a more critical eye since it is longer just a
quick-n-dirty proof of concept ...
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 3b64a97f6091..25d63731b0e0 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -196,6 +196,12 @@ struct audit_context {
> struct {
> char *name;
> } module;
> + struct {
> + struct audit_ntp_data data;
> + } ntp;
> + struct {
> + struct timespec64 injoffset;
> + } tk;
With the ntp and tk structs being separate parts of a union, are we
going to have a problem when ADJ_SETOFFSET is set in a call to
do_adjtimex()?
> };
> int fds[2];
> struct audit_proctitle proctitle;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 6efb0bb909d0..8983790ad86a 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1210,11 +1210,18 @@ static void audit_log_fcaps(struct audit_buffer *ab,
> struct audit_names *name)
> from_kuid(&init_user_ns, name->fcap.rootid));
> }
>
> +void __audit_ntp_log_(const struct audit_ntp_data *ad);
> +
> static void show_special(struct audit_context *context, int *call_panic)
> {
> struct audit_buffer *ab;
> int i;
>
> + if (context->type == AUDIT_TIME_ADJNTPVAL) {
> + __audit_ntp_log_(&context->ntp.data);
> + return;
> + }
Can we find a way to move this down into the main switch statement in
show_special() like you did with AUDIT_TIME_INJOFFSET? This looks
*really* hacky to me. Why should AUDIT_TIME_ADJNTPVAL be different
from the other "special" bits?
> ab = audit_log_start(context, GFP_KERNEL, context->type);
> if (!ab)
> return;
> @@ -1324,6 +1331,11 @@ static void show_special(struct audit_context
> *context, int *call_panic)
> audit_log_format(ab, "(null)");
>
> break;
> + case AUDIT_TIME_INJOFFSET:
> + audit_log_format(ab, "sec=%lli nsec=%li",
> + (long long)context->tk.injoffset.tv_sec,
> + context->tk.injoffset.tv_nsec);
> + break;
> }
> audit_log_end(ab);
> }
> @@ -2571,9 +2583,18 @@ void __audit_fanotify(unsigned int response)
>
> void __audit_tk_injoffset(struct timespec64 offset)
> {
> - audit_log(audit_context(), GFP_KERNEL, AUDIT_TIME_INJOFFSET,
> - "sec=%lli nsec=%li",
> - (long long)offset.tv_sec, offset.tv_nsec);
> + struct audit_context *context = audit_context();
> +
> + context->type = AUDIT_TIME_INJOFFSET;
> + memcpy(&context->tk.injoffset, &offset, sizeof(offset));
> +}
> +
> +void __audit_ntp_log(const struct audit_ntp_data *ad)
> +{
> + struct audit_context *context = audit_context();
> +
> + context->type = AUDIT_TIME_ADJNTPVAL;
> + memcpy(&context->ntp.data, ad, sizeof(*ad));
> }
>
> static void audit_log_ntp_val(const struct audit_ntp_data *ad,
> @@ -2588,7 +2609,7 @@ static void audit_log_ntp_val(const struct
> audit_ntp_data *ad,
> "op=%s old=%lli new=%lli", op, val->oldval, val->newval);
> }
>
> -void __audit_ntp_log(const struct audit_ntp_data *ad)
> +void __audit_ntp_log_(const struct audit_ntp_data *ad)
> {
> audit_log_ntp_val(ad, "offset", AUDIT_NTP_OFFSET);
> audit_log_ntp_val(ad, "freq", AUDIT_NTP_FREQ);
Ooof, *please* don't end a function, or any symbol for that matter,
with an underscore.
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
