Re: [RFC PATCH ghak10 v6 1/2] timekeeping: Audit clock adjustments

From: Thomas Gleixner
Date: Wed Mar 27 2019 - 19:37:21 EST


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.
*/

> +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.

Thanks,

tglx