linux-kernel - Re: [PATCH] clocksource: use clocksource

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

Message-ID: <CALAqxLUO4+H7JSOd=OqeUQsXmzDu4L-p=Pa3Df+jGz5WFpxrLw@mail.gmail.com>
Date:	Thu, 7 Apr 2016 21:55:32 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	Alexander Kuleshov <kuleshovmail@...il.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] clocksource: use clocksource_freq2mult() helper

On Wed, Mar 16, 2016 at 3:21 AM, Alexander Kuleshov
<kuleshovmail@...il.com> wrote:
> which is introduced in the 7aca0c072 commit to simplify calculation of
> the mult and shift in the clocks_calc_mult_shift().
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@...il.com>
> ---
>  kernel/time/clocksource.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 56ece14..de57923 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -80,9 +80,7 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 maxsec)
>          * accuracy and fits the maxsec conversion range:
>          */
>         for (sft = 32; sft > 0; sft--) {
> -               tmp = (u64) to << sft;
> -               tmp += from / 2;
> -               do_div(tmp, from);
> +               tmp = clocksource_freq2mult(from, sft, to);
>                 if ((tmp >> sftacc) == 0)
>                         break;
>         }

I'm worried you never tested this, as its clearly broken, and keeps my
systems from booting.

clocksource_freq2mult returns a u32. In the code being removed, tmp is
a u64. So this may truncate the high bits.

Since sftacc is often 32, this causes it to exit prematurely on the
first pass through the loop.

Please do make sure to boot test what you send out. I spent some time
thinking I had broken my qemu testing setup before I realized it was
this simple looking patch.

thanks
-john