| Message ID | 233d81a0a1e7b8eca1907998152ee848159b8774.1641659630.git.luto@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm, sched: Rework lazy mm handling | expand |
Hi Andy, On Sat, Jan 8, 2022 at 8:44 AM Andy Lutomirski <luto@kernel.org> 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@google.com> > Cc: Valentin Schneider <valentin.schneider@arm.com> > Cc: Sami Tolvanen <samitolvanen@google.com> > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > 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@arm.com/ Sami
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 */ }
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@google.com> Cc: Valentin Schneider <valentin.schneider@arm.com> Cc: Sami Tolvanen <samitolvanen@google.com> Signed-off-by: Andy Lutomirski <luto@kernel.org> --- kernel/cpu.c | 3 +++ kernel/sched/core.c | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-)