Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180824120001.20771-1-omosnace@redhat.com> <20180824120001.20771-2-omosnace@redhat.com>
 <CAHC9VhTZkrmgt7rxNYWTgYvm+TVGgRD6iFu9dDtZ7eYNXBKqHQ@mail.gmail.com>
 <CAFqZXNuir-8TE7WUHYhw5t446THO=sCjOXv+WAxNDs79e6OTWQ@mail.gmail.com>
 <CAHC9VhQ8dRTzC8vBsUOm8o4LQxVRiwgkSb23xEZwSDfsXhY3uw@mail.gmail.com> <CAFqZXNtqe8fF318pzmxzGaZgO0Mb_cacNKTRRbFNyMFhxUspJg@mail.gmail.com>
In-Reply-To: <CAFqZXNtqe8fF318pzmxzGaZgO0Mb_cacNKTRRbFNyMFhxUspJg@mail.gmail.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Sat, 22 Sep 2018 16:42:35 -0400
Message-ID: <CAHC9VhSoZD1uFX5FmUbLWeSPQS9ii2hyQAKV2yM8PeV5dHm-8Q@mail.gmail.com>
Subject: Re: [PATCH ghak10 v5 1/2] audit: Add functions to log time adjustments
To:     omosnace@redhat.com
Cc:     linux-audit@redhat.com, rgb@redhat.com, sgrubb@redhat.com,
        mlichvar@redhat.com, john.stultz@linaro.org, tglx@linutronix.de,
        sboyd@kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Sep 21, 2018 at 7:21 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Mon, Sep 17, 2018 at 4:51 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Mon, Sep 17, 2018 at 8:38 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > On Fri, Sep 14, 2018 at 5:19 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > On Fri, Aug 24, 2018 at 8:00 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:

...

> > Okay, with that in mind, perhaps when recording the offset values we
> > omit the "old" values (arguably that doesn't make much sense here) and
> > keep the sec/nsec split:
> >
> > type=TIME_CHANGE [...]: offset_sec=<X> offset_nsec=<Y>
> >
> > ... and for all others we stick with:
> >
> > type=TIME_CHANGE [...]: ntp_<VAR>=<NEWVAL> old=<OLD_VAL>
>
> Alright, that format would work. However, I would still like to have a
> separate type for the offset injection, since it has different field
> structure and semantics (difference vs. new+old). I don't see any
> reason to sacrifice the distinction for just one record type slot
> (AFAIK we technically still have about 2 billion left...).
>
> (Maybe you just duplicated the record type by mistake, in that case
> please disregard the last sentence.)

A reasonable guess on the typo, but no that was intentional :)

As described above, there is no set field ordering for the TIME_CHANGE
record, just like there is not set field ordering for the
CONFIG_CHANGE record.  Why?  We only include the state variables that
are being changed instead of including all of the available state
variables.  Yes, historically there are the "new" and "old" fields,
but I don't see that as a strong convention; the special "old=" field
name helps prevent confusion.

Yes, we aren't really at risk of running out of record types, but why
do we *need* two types here?  I don't believe the ordering/structure
argument is significant in this particular case, and I would much
rather see all the time related state changes included in one
TIME_CHANGE record.

-- 
paul moore
www.paul-moore.com