lkml.org 
[lkml]   [2019]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC PATCH ghak10 v6 1/2] timekeeping: Audit clock adjustments
On Thu, Mar 28, 2019 at 1:09 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 7 Mar 2019, Ondrej Mosnacek wrote:
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2512,6 +2512,14 @@ void __audit_fanotify(unsigned int response)
> > AUDIT_FANOTIFY, "resp=%u", response);
> > }
> >
> > +/* We need to allocate with GFP_ATOMIC here, since these two functions will be
> > + * called while holding the timekeeping lock: */
>
> Audit is no justification for doing ATOMIC allocations just because it's
> convenient in the middle of code which blocks every concurrent reader.
>
> Please find a place outside of the timekeeper lock to do that audit
> logging. Either that or allocate your buffer upfront in a preemptible
> section and commit after the critical section.
>
> /*
> * Aside of that please use proper multiline comment style and not this
> * horrible other one.
> */

Oh, sorry, I wrote that code last summer, when I didn't quite have the
kernel coding style in my blood yet :) But fortunately I shouldn't
need that comment at all in the next version...

>
> > +void __audit_tk_injoffset(struct timespec64 offset)
> > +{
> > + audit_log(audit_context(), GFP_ATOMIC, AUDIT_TIME_INJOFFSET,
> > + "sec=%lli nsec=%li", (long long)offset.tv_sec, offset.tv_nsec);
> > +}
> > +
> > @@ -1250,6 +1251,9 @@ out:
> > /* signal hrtimers about time change */
> > clock_was_set();
> >
> > + if (!ret)
> > + audit_tk_injoffset(ts_delta);
>
> This one does not need GFP_ATOMIC at all.
>
> > +
> > return ret;
> > }
> > EXPORT_SYMBOL(do_settimeofday64);
> > @@ -2322,6 +2326,8 @@ int do_adjtimex(struct timex *txc)
> > ret = timekeeping_inject_offset(&delta);
> > if (ret)
> > return ret;
> > +
> > + audit_tk_injoffset(delta);
> > }
> >
> > ktime_get_real_ts64(&ts);
>
> This can be done at the end of do_adjtimex() quite nicely in preemptible
> context.

But wait, isn't this call outside of the critical section as well? (I
must have been moving the call around when I was writing the code and
didn't realize that this function actually doesn't need GFP_ATOMIC at
all...) Or am I missing something?


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

\
 
 \ /
  Last update: 2019-04-01 11:18    [W:0.100 / U:22.852 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site