Messages in this thread | | | From | Woody Lin <> | Date | Tue, 12 Oct 2021 18:35:48 +0800 | Subject | Re: [PATCH] sched/scs: Reset the shadow stack when idle_task_exit |
| |
On Tue, Oct 12, 2021 at 6:00 PM Valentin Schneider <valentin.schneider@arm.com> wrote: > > On 12/10/21 16:35, Woody Lin wrote: > > There was a 'init_idle' that resets scs sp to base, but is removed by > > f1a0a376ca0c. Without the resetting, the hot-plugging implemented by > > cpu_psci_cpu_boot will use the previous scs sp as new base when starting > > up a CPU core, so the usage on scs page is being stacked up until > > overflow. > > > > This only happens on idle task since __cpu_up is using idle task as the > > main thread to start up a CPU core, so the overflow can be fixed by > > resetting scs sp to base in 'idle_task_exit'. > > > > Looking at init_idle() for similar issues, it looks like we might also want > to re-issue kasan_unpoison_task_stack() on the idle task upon hotplug. > > > Fixes: f1a0a376ca0c ("sched/core: Initialize the idle task with preemption disabled") > > Signed-off-by: Woody Lin <woodylin@google.com> > > --- > > kernel/sched/core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index 1bba4128a3e6..f21714ea3db8 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -8795,6 +8795,7 @@ 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 */ > > So AIUI for SCS that works just fine - one thing I'm unclear on is how the > following pops are going to work given the SP reset happens in the middle > of a call stack, but AFAICT that was already the case before I messed about > with init_idle(), so that must already be handled.
Hi Valentin,
Thanks for the question. The 'scs_task_reset' here resets only the '.thread_info.scs_sp' of the task, so the register (on arm64 it's x18) is still pointing to the same location for popping and storing call frames. The register will be updated to '.thread_info.scs_sp' in '__secondary_switched', which starts a new core and there is no popping after the updating, so it won't introduce an underflow.
> > I'm not familiar enough with KASAN to say whether that > kasan_unpoison_task_stack() should rather happen upon hotplugging the CPU > back (rather than on hot-unplug). If that is the case, then maybe somewhere > around cpu_startup_entry() might work (and then you could bunch these two > "needs to be re-run at init for the idle task" functions into a common > helper).
unpoison looks more like an one-time thing to me; the idle tasks will reuse the same stack pages until system resets, so I think we don't need to re-unpoison that during hotplugging as long as it's unpoisoned in 'init_idle'.
> > > } > > > > -- > > 2.33.0.882.g93a45727a2-goog
| |