Re: [PATCH ghak10 v5 1/2] audit: Add functions to log time adjustments

On 2018-09-13 23:18, Paul Moore wrote: > On Fri, Aug 24, 2018 at 8:00 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > This patch adds two auxiliary record types that will be used to annotate > > the adjtimex SYSCALL records with the NTP/timekeeping values that have > > been changed. > > > > Next, it adds two functions to the audit interface: > > - audit_tk_injoffset(), which will be called whenever a timekeeping > > offset is injected by a syscall from userspace, > > - audit_ntp_adjust(), which will be called whenever an NTP internal > > variable is changed by a syscall from userspace. > > > > Quick reference for the fields of the new records: > > AUDIT_TIME_INJOFFSET > > sec - the 'seconds' part of the offset > > nsec - the 'nanoseconds' part of the offset > > AUDIT_TIME_ADJNTPVAL > > 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 > > I understand that reusing "op" is tempting, but the above aren't > really operations, they are state variables which are being changed. > Using the CONFIG_CHANGE record as a basis, I wonder if we are better > off with something like the following: > > type=TIME_CHANGE <var>=<value_new> old=<value_old> > > ... you might need to preface the variable names with something like > "ntp_" or "offset_". You'll notice I'm also suggesting we use a > single record type here; is there any reason why two records types are > required? Why not do something like: type=TIME_CHANGE var=<var> new=<value_new> old=<value_old> So that we don't pollute the field namespace *and* create 8 variants on the same record format? This shouldn't be much of a concern with binary record formats, but we're stuck with the current parsing scheme for now.