From: Thomas Gleixner <tglx@linutronix.de>
To: Ondrej Mosnacek <omosnace@redhat.com>
Cc: linux-audit@redhat.com, Paul Moore <paul@paul-moore.com>,
Richard Guy Briggs <rgb@redhat.com>,
Steve Grubb <sgrubb@redhat.com>,
Miroslav Lichvar <mlichvar@redhat.com>,
John Stultz <john.stultz@linaro.org>,
Stephen Boyd <sboyd@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH ghak10 v6 2/2] ntp: Audit NTP parameters adjustment
Date: Thu, 28 Mar 2019 01:02:36 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.21.1903280039530.1789@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20190307123254.348-3-omosnace@redhat.com>
On Thu, 7 Mar 2019, Ondrej Mosnacek wrote:
> Emit an audit record every time selected NTP parameters are modified
> from userspace (via adjtimex(2) or clock_adjtime(2)).
>
> Such events will now generate records of type AUDIT_TIME_ADJNTPVAL
> containing the following fields:
> - op -- which value was adjusted:
> - offset -- corresponding to the time_offset variable
> - freq -- corresponding to the time_freq variable
> - status -- corresponding to the time_status variable
> - adjust -- corresponding to the time_adjust variable
> - tick -- corresponding to the tick_usec variable
> - tai -- corresponding to the timekeeping's TAI offset
> - old -- the old value
> - new -- the new value
>
> For reference, running the following commands:
>
> auditctl -D
> auditctl -a exit,always -F arch=b64 -S adjtimex
> chronyd -q
>
> produces audit records like this:
>
> type=SYSCALL msg=audit(1530616044.507:5): arch=c000003e syscall=159 success=yes exit=5 a0=7fff57e78c00 a1=0 a2=4 a3=7f754ae28c0a items=0 ppid=626 pid=629 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=1 comm="chronyd" exe="/usr/sbin/chronyd" subj=system_u:system_r:kernel_t:s0 key=(null)
<SNIP gazillions of lines of unparseable garbage>
Is it really necessary to put this into the changelog?
>
> +void __audit_ntp_adjust(const char *type, s64 oldval, s64 newval)
> +{
> + audit_log(audit_context(), GFP_ATOMIC, AUDIT_TIME_ADJNTPVAL,
No.
> + "op=%s old=%lli new=%lli", type,
> + (long long)oldval, (long long)newval);
> +}
> +
> static void audit_log_task(struct audit_buffer *ab)
> {
> kuid_t auid, uid;
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index 36a2bef00125..5f456a84151a 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -17,6 +17,7 @@
> #include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/rtc.h>
> +#include <linux/audit.h>
>
> #include "ntp_internal.h"
> #include "timekeeping_internal.h"
> @@ -293,6 +294,8 @@ static inline s64 ntp_update_offset_fll(s64 offset64, long secs)
>
> static void ntp_update_offset(long offset)
> {
> + s64 old_offset = time_offset;
> + s64 old_freq = time_freq;
> s64 freq_adj;
> s64 offset64;
> long secs;
> @@ -341,6 +344,9 @@ static void ntp_update_offset(long offset)
> time_freq = max(freq_adj, -MAXFREQ_SCALED);
>
> time_offset = div_s64(offset64 << NTP_SCALE_SHIFT, NTP_INTERVAL_FREQ);
> +
> + audit_ntp_adjust("offset", old_offset, time_offset);
> + audit_ntp_adjust("freq", old_freq, time_freq);
> }
>
> /**
> @@ -658,21 +664,31 @@ static inline void process_adj_status(const struct timex *txc)
>
> static inline void process_adjtimex_modes(const struct timex *txc, s32 *time_tai)
> {
> - if (txc->modes & ADJ_STATUS)
> - process_adj_status(txc);
> + if (txc->modes & (ADJ_STATUS | ADJ_NANO | ADJ_MICRO)) {
> + int old_status = time_status;
> +
> + if (txc->modes & ADJ_STATUS)
> + process_adj_status(txc);
> - if (txc->modes & ADJ_NANO)
> - time_status |= STA_NANO;
> + if (txc->modes & ADJ_NANO)
> + time_status |= STA_NANO;
>
> - if (txc->modes & ADJ_MICRO)
> - time_status &= ~STA_NANO;
> + if (txc->modes & ADJ_MICRO)
> + time_status &= ~STA_NANO;
> +
> + audit_ntp_adjust("status", old_status, time_status);
> + }
>
> if (txc->modes & ADJ_FREQUENCY) {
> + s64 old_freq = time_freq;
> +
> time_freq = txc->freq * PPM_SCALE;
> time_freq = min(time_freq, MAXFREQ_SCALED);
> time_freq = max(time_freq, -MAXFREQ_SCALED);
> /* update pps_freq */
> pps_set_freq(time_freq);
> +
> + audit_ntp_adjust("freq", old_freq, time_freq);
> }
>
> if (txc->modes & ADJ_MAXERROR)
> @@ -689,14 +705,18 @@ static inline void process_adjtimex_modes(const struct timex *txc, s32 *time_tai
> time_constant = max(time_constant, 0l);
> }
>
> - if (txc->modes & ADJ_TAI && txc->constant > 0)
> + if (txc->modes & ADJ_TAI && txc->constant > 0) {
> + audit_ntp_adjust("tai", *time_tai, txc->constant);
> *time_tai = txc->constant;
> + }
>
> if (txc->modes & ADJ_OFFSET)
> ntp_update_offset(txc->offset);
>
> - if (txc->modes & ADJ_TICK)
> + if (txc->modes & ADJ_TICK) {
> + audit_ntp_adjust("tick", tick_usec, txc->tick);
> tick_usec = txc->tick;
> + }
>
> if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
> ntp_update_frequency();
> @@ -718,6 +738,8 @@ int __do_adjtimex(struct timex *txc, const struct timespec64 *ts, s32 *time_tai)
> /* adjtime() is independent from ntp_adjtime() */
> time_adjust = txc->offset;
> ntp_update_frequency();
> +
> + audit_ntp_adjust("adjust", save_adjust, txc->offset);
> }
> txc->offset = save_adjust;
> } else {
Not going to happen. We are not reshuffling all that code just to
accomodate random audit log invocations in a critical section plus having a
gazillion of GFP_ATOMIC allocation in the critical section just because.
The whole information can be reconstructed after the fact:
1) Copy the user space supplied struct timex to a buffer
2) Retrieve the current timex information _before_ invoking
do_adjtimex().
3) Look at the ret value and the resulting struct timex which is going
to be copied back to user space and figure out with the help of #1
and #2 what you need to log.
That does not even need a single line of change in the NTP code and almost
everything happens in fully preemptible context.
Thanks,
tglx
next prev parent reply other threads:[~2019-03-28 0:02 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-07 12:32 [RFC PATCH ghak10 v6 0/2] audit: Log changes that can affect the system clock Ondrej Mosnacek
2019-03-07 12:32 ` [RFC PATCH ghak10 v6 1/2] timekeeping: Audit clock adjustments Ondrej Mosnacek
2019-03-08 17:57 ` Steve Grubb
2019-03-27 23:26 ` John Stultz
2019-04-01 9:15 ` Ondrej Mosnacek
2019-03-27 23:37 ` Thomas Gleixner
2019-03-28 0:10 ` Thomas Gleixner
2019-03-28 0:24 ` Thomas Gleixner
2019-04-01 9:16 ` Ondrej Mosnacek
2019-04-02 9:06 ` Thomas Gleixner
2019-03-07 12:32 ` [RFC PATCH ghak10 v6 2/2] ntp: Audit NTP parameters adjustment Ondrej Mosnacek
2019-03-08 17:59 ` Steve Grubb
2019-03-27 23:29 ` John Stultz
2019-03-28 0:02 ` Thomas Gleixner [this message]
2019-04-01 9:13 ` Ondrej Mosnacek
2019-04-02 9:03 ` Thomas Gleixner
2019-04-02 15:02 ` Ondrej Mosnacek
2019-03-08 20:25 ` [RFC PATCH ghak10 v6 0/2] audit: Log changes that can affect the system clock Richard Guy Briggs
2019-03-11 11:48 ` Ondrej Mosnacek
2019-03-25 14:50 ` Paul Moore
2019-03-27 23:00 ` Paul Moore
2019-04-01 9:21 ` Ondrej Mosnacek
2019-03-27 23:36 ` John Stultz
2019-03-28 0:03 ` Thomas Gleixner
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=alpine.DEB.2.21.1903280039530.1789@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=john.stultz@linaro.org \
--cc=linux-audit@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mlichvar@redhat.com \
--cc=omosnace@redhat.com \
--cc=paul@paul-moore.com \
--cc=rgb@redhat.com \
--cc=sboyd@kernel.org \
--cc=sgrubb@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 NNTP newsgroup(s).