Hi Andy,
On Sat, Jan 8, 2022 at 8:44 AM Andy Lutomirski <luto@xxxxxxxxxx> wrote:
>
> Starting with commit 63acd42c0d49 ("sched/scs: Reset the shadow stack when
> idle_task_exit"), the idle thread's shadow stack was reset from the idle
> task's context during CPU hot-unplug. This was fragile: between resetting
> the shadow stack and actually stopping the idle task, the shadow stack
> did not match the actual call stack.
>
> Clean this up by resetting the idle task's SCS in bringup_cpu().
>
> init_idle() still does scs_task_reset() -- see the comments there. I
> leave this to an SCS maintainer to untangle further.
>
> Cc: Woody Lin <woodylin@xxxxxxxxxx>
> Cc: Valentin Schneider <valentin.schneider@xxxxxxx>
> Cc: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
> kernel/cpu.c | 3 +++
> kernel/sched/core.c | 9 ++++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 192e43a87407..be16816bb87c 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -33,6 +33,7 @@
> #include <linux/slab.h>
> #include <linux/percpu-rwsem.h>
> #include <linux/cpuset.h>
> +#include <linux/scs.h>
>
> #include <trace/events/power.h>
> #define CREATE_TRACE_POINTS
> @@ -587,6 +588,8 @@ static int bringup_cpu(unsigned int cpu)
> struct task_struct *idle = idle_thread_get(cpu);
> int ret;
>
> + scs_task_reset(idle);
> +
> /*
> * Some architectures have to walk the irq descriptors to
> * setup the vector space for the cpu which comes online.
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 917068b0a145..acd52a7d1349 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8621,7 +8621,15 @@ void __init init_idle(struct task_struct *idle, int cpu)
> idle->flags |= PF_IDLE | PF_KTHREAD | PF_NO_SETAFFINITY;
> kthread_set_per_cpu(idle, cpu);
>
> + /*
> + * NB: This is called from sched_init() on the *current* idle thread.
> + * This seems fragile if not actively incorrect.
> + *
> + * Initializing SCS for about-to-be-brought-up CPU idle threads
> + * is in bringup_cpu(), but that does not cover the boot CPU.
> + */
> scs_task_reset(idle);
> +
> kasan_unpoison_task_stack(idle);
>
> #ifdef CONFIG_SMP
> @@ -8779,7 +8787,6 @@ void idle_task_exit(void)
> finish_arch_post_lock_switch();
> }
>
> - scs_task_reset(current);
> /* finish_cpu(), as ran on the BP, will clean up the active_mm state */
> }
I believe Mark already fixed this one here:
https://lore.kernel.org/lkml/20211123114047.45918-1-mark.rutland@xxxxxxx/
Sami
