Re: [PATCH] powerpc/kernel: Avoid memory corruption at early stage

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Thu, Jan 22, 2015 at 05:21:22PM +1100, Michael Ellerman wrote:
>On Thu, 2015-01-08 at 16:40 +1100, Gavin Shan wrote:
>> When calling to early_setup(), we picks "boot_paca" up for the
>> master CPU and initialize that with initialise_paca(). At the
>> point, SLB shadow buffer isn't populated yet. Updating the SLB
>> shadow buffer should corrupt what we had in physical address 0
>> where the trap instruction is usually stored.
>
>Ouch.
>
>Introduced in 6f4441ef7009 ("powerpc: Dynamically allocate slb_shadow from
>memblock") - December 2013.
>
>So it seems it doesn't cause us any harm in general.
>
>Did you actually hit a bug with it?
>

I didn't hit any bugs with it. So I guess it's unnecessary to mark it as
"stable". I found the issue (not bug) occasionally: starting kernel in
simulator, dumping the instruction at physical address 0x0 and found
it's not "nop".

>> diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
>> index d6e195e..048a6ee 100644
>> --- a/arch/powerpc/kernel/paca.c
>> +++ b/arch/powerpc/kernel/paca.c
>> @@ -115,6 +115,9 @@ static struct slb_shadow * __init init_slb_shadow(int cpu)
>>  {
>>  	struct slb_shadow *s = &slb_shadow[cpu];
>>  
>> +	if (!slb_shadow)
>> +		return NULL;
>> +
>>  	s->persistent = cpu_to_be32(SLB_NUM_BOLTED);
>>  	s->buffer_length = cpu_to_be32(sizeof(*s));
>
>
>Yeah I guess that's an OK fix.
>
>We must have a valid SLB shadow before we ever call _switch(), which is much
>later. The only way we could hit this case for the real paca is if
>allocate_slb_shadows() failed to allocate, but it would have panicked if it
>did.
>

Perhaps I can add BUG_ON(!slb_shadow) in allocate_slb_shadows() if you agree.

Thanks,
Gavin

>cheers
>
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev