linux-kernel - Re: [PATCH 05/10] cpuidle: Comment about timers requirements VS idle handler

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, 11 Aug 2023 19:38:40 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Anna-Maria Behnsen <anna-maria@...utronix.de>
Subject: Re: [PATCH 05/10] cpuidle: Comment about timers requirements VS idle handler

On Fri, Aug 11, 2023 at 7:01 PM Frederic Weisbecker <frederic@...nel.org> wrote:
>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>

Acked-by: Rafael J. Wysocki <rafael@...nel.org>

> ---
>  kernel/sched/idle.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 342f58a329f5..d52f6e3e3854 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -258,6 +258,36 @@ static void do_idle(void)
>         while (!need_resched()) {
>                 rmb();
>
> +               /*
> +                * Interrupts shouldn't be re-enabled from that point on until
> +                * the CPU sleeping instruction is reached. Otherwise an interrupt
> +                * may fire and queue a timer that would be ignored until the CPU
> +                * wakes from the sleeping instruction. And testing need_resched()
> +                * doesn't tell about pending needed timer reprogram.
> +                *
> +                * Several cases to consider:
> +                *
> +                * - SLEEP-UNTIL-PENDING-INTERRUPT based instructions such as
> +                *   "wfi" or "mwait" are fine because they can be entered with
> +                *   interrupt disabled.
> +                *
> +                * - sti;mwait() couple is fine because the interrupts are
> +                *   re-enabled only upon the execution of mwait, leaving no gap
> +                *   in-between.
> +                *
> +                * - ROLLBACK based idle handlers with the sleeping instruction
> +                *   called with interrupts enabled are NOT fine. In this scheme
> +                *   when the interrupt detects it has interrupted an idle handler,
> +                *   it rolls back to its beginning which performs the
> +                *   need_resched() check before re-executing the sleeping
> +                *   instruction. This can leak a pending needed timer reprogram.
> +                *   If such a scheme is really mandatory due to the lack of an
> +                *   appropriate CPU sleeping instruction, then a FAST-FORWARD
> +                *   must instead be applied: when the interrupt detects it has
> +                *   interrupted an idle handler, it must resume to the end of
> +                *   this idle handler so that the generic idle loop is iterated
> +                *   again to reprogram the tick.
> +                */
>                 local_irq_disable();
>
>                 if (cpu_is_offline(cpu)) {
> --
> 2.34.1
>