[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86: re-add stack alignment check



On 14/11/16 14:24, Jan Beulich wrote:
>>>> On 14.11.16 at 14:45, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 14/11/16 13:25, Jan Beulich wrote:
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -643,6 +643,11 @@ void load_system_tables(void)
>>>             .limit = (IDT_ENTRIES * sizeof(idt_entry_t)) - 1,
>>>     };
>>>  
>>> +   /* Bottom-of-stack must be 16-byte aligned! */
>>> +   BUILD_BUG_ON((sizeof(struct cpu_info) -
>>> +                 offsetof(struct cpu_info, guest_cpu_user_regs.es)) & 0xf);
>>> +   BUG_ON(system_state != SYS_STATE_early_boot && (stack_bottom & 0xf));
>> This will still triple fault the system if it triggers on an AP. 
>> Exceptions aren't hooked up yet.
> Hmm, true. They could be moved to the very end of the function
> though?

That would avoid the triple fault, but doesn't make the BUG_ON() useful.

>
>> The reason I dropped the check was that there was no way it was going to
>> fail on the BSP (because of the alignment of cpu0_stack) or APs (because
>> of the alloc_xenheap_pages(STACK_ORDER, memflags) allocation).
> These being page aligned has nothing to do with the BUG_ON()
> triggering. I found its dropping being questionable in the context of
> the old discussion rooted at this patch of mine
> https://lists.xenproject.org/archives/html/xen-devel/2010-07/msg00642.html
> I'd like to particularly point you to the various dependencies which
> weren't properly spelled out or enforced back then. Specifically
> it (not) triggering depends on the number and type of fields in
> struct cpu_info following the guest_cpu_user_regs field.

get_stack_bottom() takes the stack pointer, or's it up to the 8-page
boundary, overlays a struct cpu_info, then returns the address of es.

There is no value of %rsp which will ever cause it to fail.  The only
think which will cause a failure is the layout of struct cpu_info, but
the BUILD_BUG_ON() will catch that.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.