linux-kernel - Re: [Patch] Fix the possibility of insane return value of hpet

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]

Message-ID: <7d1d9c250903151256l36586764wfbd5e2578a45663c@mail.gmail.com>
Date:	Sun, 15 Mar 2009 15:56:52 -0400
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	Yasunori Goto <y-goto@...fujitsu.com>
Cc:	clemens@...isch.de, Linux Kernel ML <linux-kernel@...r.kernel.org>,
	robert.picco@...com, venkatesh.pallipadi@...el.com,
	vojtech@...e.cz, mingo@...hat.com
Subject: Re: [Patch] Fix the possibility of insane return value of 
	hpet_calibrate() against SMI.

On Fri, Mar 13, 2009 at 1:00 AM, Yasunori Goto <y-goto@...fujitsu.com> wrote:
> Hello.
>
> I think there is a possibility that HPET driver will return
> insane value due to a SMI interruption (or switching guests by hypervisor).
> I found it by reviewing, and I would like to fix it.
>
> Current HPET driver calibrates the adjustment value
> by calculation the elapse time in CPU busy loop.
> However this way is too dangerous against SMI interruption.
>
> Here is the calibration code in hpet_calibrate()
>
>  701 static unsigned long hpet_calibrate(struct hpets *hpetp)
>             :
>             :
>  728         do {
>  729                 m = read_counter(&hpet->hpet_mc);
>  730                 write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
>  731         } while (i++, (m - start) < count);
>  732
>  733         local_irq_restore(flags);
>  734
>  735         return (m - start) / i;
>
> If SMI interruption occurs between 728 to 731, then return value will be
> bigger value than correct one. (SMI is not able to be controlled by OS.)
>
>
> This patch is a simple solution to fix it.
> hpet_calibrate() is called 5 times, and one of them is expected as
> correct value.

I've no sense of feel for how long each calibration run would take.
Would doing it 5 times show up as a significant increase in the boot
time for those that care about boot time being as quick as possible?

Paul.

>
> Thanks.
>
>
> ---
>
> hpet_calibrate() has a possibility of miss-calibration due to SMI.
> If SMI interrupts in the while loop of calibration, then return value
> will be big. This changes it tries 5 times and get minimum value as
> correct value.
>
> Signed-off-by: Yasunori Goto <y-goto@...fujitsu.com>
>
> ---
>
> Index: hpet_test/drivers/char/hpet.c
> ===================================================================
> --- hpet_test.orig/drivers/char/hpet.c  2008-12-04 16:24:02.000000000 +0900
> +++ hpet_test/drivers/char/hpet.c       2008-12-04 16:34:59.000000000 +0900
> @@ -713,7 +713,7 @@
>  */
>  #define        TICK_CALIBRATE  (1000UL)
>
> -static unsigned long hpet_calibrate(struct hpets *hpetp)
> +static unsigned long __hpet_calibrate(struct hpets *hpetp)
>  {
>        struct hpet_timer __iomem *timer = NULL;
>        unsigned long t, m, count, i, flags, start;
> @@ -750,6 +750,17 @@
>        return (m - start) / i;
>  }
>
> +static unsigned long hpet_calibrate(struct hpets *hpetp)
> +{
> +       unsigned long ret = ~0UL, i;
> +
> +       /* Try 5 times to remove impact of SMI.*/
> +       for (i = 0; i < 5; i++)
> +               ret = min(ret, __hpet_calibrate(hpetp));
> +
> +       return ret;
> +}
> +
>  int hpet_alloc(struct hpet_data *hdp)
>  {
>        u64 cap, mcfg;
>
> --
> Yasunori Goto
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/