[Skiboot] [PATCH] core/cpu: Prevent clobbering of stack guard for boot-cpu

[Nicholas Piggin]

On Wed,  4 Apr 2018 17:03:07 +0530
Vaibhav Jain <vaibhav at linux.vnet.ibm.com> wrote:

> Commit 90d53934c2da ("core/cpu: discover stack region size before
> initialising memory regions") introduced memzero for struct cpu_thread
> in init_cpu_thread(). This has an unintended side effect of clobbering
> the stack-guard cannery of the boot_cpu stack. This results in opal
> failing to init with this failure message:
> 
> CPU: P9 generation processor (max 4 threads/core)
> CPU: Boot CPU PIR is 0x0004 PVR is 0x004e1200
> Guard skip = 0
> Stack corruption detected !
> Aborting!
> CPU 0004 Backtrace:
>  S: 0000000031c13ab0 R: 0000000030013b0c   .backtrace+0x5c
>  S: 0000000031c13b50 R: 000000003001bd18   ._abort+0x60
>  S: 0000000031c13be0 R: 0000000030013bbc   .__stack_chk_fail+0x54
>  S: 0000000031c13c60 R: 00000000300c5b70   .memset+0x12c
>  S: 0000000031c13d00 R: 0000000030019aa8   .init_cpu_thread+0x40
>  S: 0000000031c13d90 R: 000000003001b520   .init_boot_cpu+0x188
>  S: 0000000031c13e30 R: 0000000030015050   .main_cpu_entry+0xd0
>  S: 0000000031c13f00 R: 0000000030002700   boot_entry+0x1c0
> 
> So the patch provides a fix by tweaking the memset() call in
> init_cpu_thread() to skip over the stack-guard cannery.
> 
> Fixes:90d53934c2da("core/cpu: discover stack region size before initialising memory regions")
> Signed-off-by: Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
>

Thanks for the patch, looks good to me.

Reviewed-by: Nicholas Piggin <npiggin at gmail.com>

 ---
>  core/cpu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/core/cpu.c b/core/cpu.c
> index 1a7ccb0b..6826fee0 100644
> --- a/core/cpu.c
> +++ b/core/cpu.c
> @@ -826,7 +826,11 @@ static void init_cpu_thread(struct cpu_thread *t,
>  			    enum cpu_thread_state state,
>  			    unsigned int pir)
>  {
> -	memset(t, 0, sizeof(struct cpu_thread));
> +	/* offset within cpu_thread to prevent stack_guard clobber */
> +	const size_t guard_skip = container_off_var(t, stack_guard) +
> +		sizeof(t->stack_guard);
> +
> +	memset(t + guard_skip, 0, sizeof(struct cpu_thread) - guard_skip);
>  	init_lock(&t->dctl_lock);
>  	init_lock(&t->job_lock);
>  	list_head_init(&t->job_queue);