From: Paul Moore <paul@paul-moore.com>
To: Richard Guy Briggs <rgb@redhat.com>
Cc: Eric Paris <eparis@parisplace.org>,
Linux-Audit Mailing List <linux-audit@redhat.com>
Subject: Re: [PATCH v4] audit: log AUDIT_TIME_* records only from rules
Date: Tue, 15 Feb 2022 18:06:54 -0500 [thread overview]
Message-ID: <CAHC9VhRq1e+vEnntj5HNqR2+GmLARwo8GGHA1ndGV4tUNsgocA@mail.gmail.com> (raw)
In-Reply-To: <8581d6f4b9e732d11c258fe850666f1eefab828f.1644621720.git.rgb@redhat.com>
On Mon, Feb 14, 2022 at 10:23 AM Richard Guy Briggs <rgb@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 in the
> context and log it at syscall exit time respecting the filter rules.
>
> Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919
>
> Fixes: 7e8eda734d30 ("ntp: Audit NTP parameters adjustment")
> Fixes: 2d87a0674bd6 ("timekeeping: Audit clock adjustments")
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> Changelog:
> v2:
> - rename __audit_ntp_log_ to audit_log_ntp
> - pre-check ntp before storing
> - move tk out of the context union and move ntp logging to the bottom of audit_show_special()
> - restructure logging of ntp to use ab and allocate more only if more
> - add Fixes lines
> v3
> - move tk into union
> - rename audit_log_ntp() to audit_log_time() and add tk
> - key off both AUDIT_TIME_* but favour NTP
> v4
> - drop tk goto in favour of ntp if clause
> - add comments to clarify calling function buffer expectations
>
> kernel/audit.h | 4 +++
> kernel/auditsc.c | 86 +++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 70 insertions(+), 20 deletions(-)
...
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index fce5d43a933f..86f1c65ac933 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1340,6 +1340,53 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
> from_kuid(&init_user_ns, name->fcap.rootid));
> }
>
> +void audit_log_time(struct audit_context *context, struct audit_buffer **ab)
> +{
> + const struct audit_ntp_data *ntp = &context->time.ntp_data;
> + const struct timespec64 *tk = &context->time.tk_injoffset;
> + const char *ntp_name[] = {
> + "offset",
> + "freq",
> + "status",
> + "tai",
> + "tick",
> + "adjust",
> + };
> + /* use up allocated ab from show_special before new one */
> + int type, first = 1;
> +
> + if (context->type == AUDIT_TIME_ADJNTPVAL) {
> + for (type = 0; type < AUDIT_NTP_NVALS; type++) {
> + if (ntp->vals[type].newval != ntp->vals[type].oldval) {
> + if (first) {
> + first = 0;
> + } else {
> + audit_log_end(*ab);
> + *ab = audit_log_start(context, GFP_KERNEL,
> + AUDIT_TIME_ADJNTPVAL);
> + if (!*ab)
> + return;
> + }
> + audit_log_format(*ab, "op=%s old=%lli new=%lli",
> + ntp_name[type], ntp->vals[type].oldval,
> + ntp->vals[type].newval);
> + }
> + }
> + }
> + if (tk->tv_sec != 0 || tk->tv_nsec != 0) {
> + if (!first) {
> + audit_log_end(*ab);
> + *ab = audit_log_start(context, GFP_KERNEL,
> + AUDIT_TIME_INJOFFSET);
> + if (!*ab)
> + return;
> + }
> + audit_log_format(*ab, "sec=%lli nsec=%li",
> + (long long)tk->tv_sec, tk->tv_nsec);
> + }
> + /* allocated ab will be ended by show_special */
> +}
As discussed earlier, we can simplify this function above (e.g. get
rid of "first" and all the related work) by allowing audit_log_time()
to return a NULL audit_buffer. I'd rather see that than the
audit_log_time() implementation above.
The audit_log_end() and most (all?) of the audit_log_*() functions can
safely handle a NULL audit_buffer, if they couldn't handle that
condition the kernel would die a terrible death in other areas. We
might as well use that to our advantage here and clean up
audit_log_time().
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
prev parent reply other threads:[~2022-02-15 23:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-14 15:23 [PATCH v4] audit: log AUDIT_TIME_* records only from rules Richard Guy Briggs
2022-02-15 23:06 ` Paul Moore [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHC9VhRq1e+vEnntj5HNqR2+GmLARwo8GGHA1ndGV4tUNsgocA@mail.gmail.com \
--to=paul@paul-moore.com \
--cc=eparis@parisplace.org \
--cc=linux-audit@redhat.com \
--cc=rgb@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).