Message ID | 285fcf8852b5924cb01de00be1152ea617527c52.1539944940.git.christophe.leroy@c-s.fr (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v2] idle/x86: remove the call to boot_init_stack_canary() from cpu_startup_entry() | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
On 10/19/2018 03:31 AM, Christophe Leroy wrote: > commit d7880812b359 ("idle: Add the stack canary init to > cpu_startup_entry()") added the call to boot_init_stack_canary() > in cpu_startup_entry() in an #ifdef CONFIG_X86 statement, with > the intention to remove that #ifdef later. > > While implementing stack protector for powerpc, it has been > observed that calling boot_init_stack_canary() is also needed > for powerpc which uses per task (TLS) stack canary like the X86. > > However, calling boot_init_stack_canary() would break arches > using global stack canary (ARM, SH, MIPS and XTENSA). > > Instead of modifying the #ifdef CONFIG_X86 in an > #if defined(CONFIG_X86) || defined(CONFIG_PPC), powerpc > implemented the call to boot_init_stack_canary() in the function > calling cpu_startup_entry() > > On x86, we have two functions calling cpu_startup_entry(): > - start_secondary() > - cpu_bringup_and_idle() > > start_secondary() already calls boot_init_stack_canary(). > > This patch adds the call to boot_init_stack_canary() in > cpu_bringup_and_idle() and removes it from cpu_startup_entry() > > Reviewed-by: Juergen Gross <jgross@suse.com> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > --- > v2: Revised commit log (#if defined had been droped by 'git commit') > > arch/x86/xen/smp_pv.c | 1 + > kernel/sched/idle.c | 15 --------------- > 2 files changed, 1 insertion(+), 15 deletions(-) > > diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c > index e3b18ad49889..0e05e8e23998 100644 > --- a/arch/x86/xen/smp_pv.c > +++ b/arch/x86/xen/smp_pv.c > @@ -88,6 +88,7 @@ static void cpu_bringup(void) > asmlinkage __visible void cpu_bringup_and_idle(void) > { > cpu_bringup(); > + boot_init_stack_canary(); I think I saw from the kbuild test robot in your first pass that this is an implicit declaration. Quick look at my local git tree and I don't see an include for stackprotector.h in this file. -Tyrel > cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); > } > > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > index 16f84142f2f4..f5516bae0c1b 100644 > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -347,21 +347,6 @@ EXPORT_SYMBOL_GPL(play_idle); > > void cpu_startup_entry(enum cpuhp_state state) > { > - /* > - * This #ifdef needs to die, but it's too late in the cycle to > - * make this generic (ARM and SH have never invoked the canary > - * init for the non boot CPUs!). Will be fixed in 3.11 > - */ > -#ifdef CONFIG_X86 > - /* > - * If we're the non-boot CPU, nothing set the stack canary up > - * for us. The boot CPU already has it initialized but no harm > - * in doing it again. This is a good place for updating it, as > - * we wont ever return from this function (so the invalid > - * canaries already on the stack wont ever trigger). > - */ > - boot_init_stack_canary(); > -#endif > arch_cpu_idle_prepare(); > cpuhp_online_idle(state); > while (1) >
Hi Christophe, Thank you for the patch! Yet something to improve: [auto build test ERROR on xen-tip/linux-next] [also build test ERROR on v4.19 next-20181019] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/idle-x86-remove-the-call-to-boot_init_stack_canary-from-cpu_startup_entry/20181020-061217 base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next config: x86_64-rhel (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): arch/x86/xen/smp_pv.c: In function 'cpu_bringup_and_idle': >> arch/x86/xen/smp_pv.c:91:2: error: implicit declaration of function 'boot_init_stack_canary'; did you mean 'snprint_stack_trace'? [-Werror=implicit-function-declaration] boot_init_stack_canary(); ^~~~~~~~~~~~~~~~~~~~~~ snprint_stack_trace cc1: some warnings being treated as errors vim +91 arch/x86/xen/smp_pv.c 87 88 asmlinkage __visible void cpu_bringup_and_idle(void) 89 { 90 cpu_bringup(); > 91 boot_init_stack_canary(); 92 cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); 93 } 94 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c index e3b18ad49889..0e05e8e23998 100644 --- a/arch/x86/xen/smp_pv.c +++ b/arch/x86/xen/smp_pv.c @@ -88,6 +88,7 @@ static void cpu_bringup(void) asmlinkage __visible void cpu_bringup_and_idle(void) { cpu_bringup(); + boot_init_stack_canary(); cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); } diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index 16f84142f2f4..f5516bae0c1b 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -347,21 +347,6 @@ EXPORT_SYMBOL_GPL(play_idle); void cpu_startup_entry(enum cpuhp_state state) { - /* - * This #ifdef needs to die, but it's too late in the cycle to - * make this generic (ARM and SH have never invoked the canary - * init for the non boot CPUs!). Will be fixed in 3.11 - */ -#ifdef CONFIG_X86 - /* - * If we're the non-boot CPU, nothing set the stack canary up - * for us. The boot CPU already has it initialized but no harm - * in doing it again. This is a good place for updating it, as - * we wont ever return from this function (so the invalid - * canaries already on the stack wont ever trigger). - */ - boot_init_stack_canary(); -#endif arch_cpu_idle_prepare(); cpuhp_online_idle(state); while (1)