linux-kernel - Re: [PATCH ghak10 v7 2/2] ntp: Audit NTP parameters adjustment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFqZXNvGY7ZMQ-zxa=6sm0v-VK6u9U+RzDAhiYFKCvDpDbQdrw@mail.gmail.com>
Date:   Wed, 10 Apr 2019 09:29:32 +0200
From:   Ondrej Mosnacek <omosnace@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        Paul Moore <paul@...l-moore.com>,
        Richard Guy Briggs <rgb@...hat.com>,
        Steve Grubb <sgrubb@...hat.com>,
        Miroslav Lichvar <mlichvar@...hat.com>,
        John Stultz <john.stultz@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH ghak10 v7 2/2] ntp: Audit NTP parameters adjustment

On Wed, Apr 10, 2019 at 9:03 AM Thomas Gleixner <tglx@...utronix.de> wrote:
> On Tue, 9 Apr 2019, Ondrej Mosnacek wrote:
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 2c62c0468888..1c372ad7ebe9 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -86,6 +86,26 @@ struct audit_field {
> >       u32                             op;
> >  };
> >
> > +#define AUDIT_NTP_OFFSET     0
> > +#define AUDIT_NTP_FREQ               1
> > +#define AUDIT_NTP_STATUS     2
> > +#define AUDIT_NTP_TAI                3
> > +#define AUDIT_NTP_TICK               4
> > +#define AUDIT_NTP_ADJUST     5
> > +#define AUDIT_NTP_NVALS              6 /* count */
>
> That should be an named enum and the id argument should be the same type.

I kind of instinctively went for macros to be consistent with the rest
of the file, but on second thought that's really not a good reason to
keep doing the wrong thing... I'll change it to enum in v8.

>
> > @@ -720,14 +721,29 @@ int __do_adjtimex(struct __kernel_timex *txc, const struct timespec64 *ts,
> >                       /* adjtime() is independent from ntp_adjtime() */
> >                       time_adjust = txc->offset;
> >                       ntp_update_frequency();
> > +
> > +                     audit_ntp_set_old(ad, AUDIT_NTP_ADJUST, save_adjust);
> > +                     audit_ntp_set_new(ad, AUDIT_NTP_ADJUST, time_adjust);
> >               }
> >               txc->offset = save_adjust;
> >       } else {
> > -
> >               /* If there are input parameters, then process them: */
> > -             if (txc->modes)
> > +             if (txc->modes) {
> > +                     audit_ntp_set_old(ad, AUDIT_NTP_OFFSET, time_offset);
> > +                     audit_ntp_set_old(ad, AUDIT_NTP_FREQ,   time_freq);
> > +                     audit_ntp_set_old(ad, AUDIT_NTP_STATUS, time_status);
> > +                     audit_ntp_set_old(ad, AUDIT_NTP_TAI,    *time_tai);
> > +                     audit_ntp_set_old(ad, AUDIT_NTP_TICK,   tick_usec);
> > +
> >                       process_adjtimex_modes(txc, time_tai);
> >
> > +                     audit_ntp_set_new(ad, AUDIT_NTP_OFFSET, time_offset);
> > +                     audit_ntp_set_new(ad, AUDIT_NTP_FREQ,   time_freq);
> > +                     audit_ntp_set_new(ad, AUDIT_NTP_STATUS, time_status);
> > +                     audit_ntp_set_new(ad, AUDIT_NTP_TAI,    *time_tai);
> > +                     audit_ntp_set_new(ad, AUDIT_NTP_TICK,   tick_usec);
> > +             }
>
> Yes, that looks much more palatable! Nice work!
>
> With the above addressed:
>
> Reviewed-by: Thomas Gleixner <tglx@...utronix.de>
>
>

Thanks!

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.