linux-kernel - Re: [PATCH v2 4/4] hv_util: improve time adjustment accuracy by disabling interrupts

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 6 Jan 2017 17:02:54 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
Cc:     "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Alex Ng <alexng@...rosoft.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH v2 4/4] hv_util: improve time adjustment accuracy by
 disabling interrupts

On Wed, Jan 4, 2017 at 9:24 AM, Vitaly Kuznetsov <vkuznets@...hat.com> wrote:
> If we happen to receive interrupts during hv_set_host_time() execution
> our adjustments may get inaccurate. Make the whole function atomic.
> Unfortunately, we can's call do_settimeofday64() with interrupts
> disabled as some cross-CPU work is being done but this call happens
> very rarely.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
> ---
>  drivers/hv/hv_util.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 7e97231..4e50a42 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -187,6 +187,9 @@ static void hv_set_host_time(struct work_struct *work)
>         struct timespec64 host_ts, our_ts;
>         struct timex txc = {0};
>         int ret;
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
>
>         wrk = container_of(work, struct adj_time_work, work);
>
> @@ -218,6 +221,7 @@ static void hv_set_host_time(struct work_struct *work)
>          * ordered to sync our time by the host.
>          */
>         if (abs(delta) > MAXPHASE || wrk->flags & ICTIMESYNCFLAG_SYNC) {
> +               local_irq_restore(flags);
>                 do_settimeofday64(&host_ts);
>                 return;
>         }
> @@ -232,6 +236,8 @@ static void hv_set_host_time(struct work_struct *work)
>         ret = do_adjtimex(&txc);
>         if (ret)
>                 pr_debug("Failed to adjust system time: %d\n", ret);
> +
> +       local_irq_restore(flags);


This seems like a long time to disable irqs for what your trying to
do. I'd guess you really only want to disable irqs while you aquire
the host and guest timestamps (so they are as close together as
possible).  Since the delta is then calculated, I'm not sure what
disabling irqs for calling adjtimex gets you.

thanks
-john