linux-kernel - Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable

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:	Tue, 26 Jul 2016 14:36:35 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	Kyle Walker <kwalker@...hat.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH resend] clocksource: Defer override invalidation unless
 clock is unstable

Sorry for not getting back to you. This has been in my to-look-at list.

On Tue, Jul 26, 2016 at 2:24 PM, Kyle Walker <kwalker@...hat.com> wrote:
> The clock_select() operation will attempt to use the clocksource override
> to apply the desired clocksource when the "clocksource=" boot parameter is
> supplied. However, in the event that "clocksource=tsc" is used on a system
> where there is a more desireable clocksource available, the boot parameter
> fails. This is due to the TSC clocksource being installed unvalidated, but
> the override being invalidated during the initial run through
> clocksource_done_booting().

I've read this a few times, and I'm not sure I really understand it.

Can you give an example of a "more desirable clocksource" then the
TSC?  Especially when the TSC was specified as a boot argument?


>
> To address this condition, the override_name is only invalidated for
> unstable clocksources. Otherwise, the override is left intact until after
> the watchdog has validated the clocksource as stable/unstable.
>
> Signed-off-by: Kyle Walker <kwalker@...hat.com>
> Cc: John Stultz <john.stultz@...aro.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: linux-kernel@...r.kernel.org
> ---
>
> Notes:
>     Resend due to no feedback on the initial submit. Thank you in advance!
>
>  kernel/time/clocksource.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 56ece14..4c1bb2a 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -600,9 +600,18 @@ static void __clocksource_select(bool skipcur)
>                  */
>                 if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
>                         /* Override clocksource cannot be used. */
> -                       pr_warn("Override clocksource %s is not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
> -                               cs->name);
> -                       override_name[0] = 0;
> +                       if (cs->flags & CLOCK_SOURCE_UNSTABLE) {
> +                               pr_warn("Override clocksource %s is unstable and not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
> +                                       cs->name);
> +                               override_name[0] = 0;
> +                       } else {
> +                               /*
> +                                * The override cannot be currently verified.
> +                                * Deferring to let the watchdog check.
> +                                */
> +                               pr_info("Override clocksource %s is not currently HRT compatible - deferring\n",
> +                                       cs->name);
> +                       }

The logic here is confusing as well. So.. if the override is not HRT
compatible, we check if its stable or not?  Once we're in HRT there's
not much likelyhood of us going into non HRT mode. I'm not sure what
the stability has to do with it here.

Sorry, could you explain the case you're running into in some further detail?

thanks
-john