Re: [Xen-devel] [PATCH v2 2/2] x86/xpti: Don't leak TSS-adjacent percpu data via Meltdown

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 29/07/2019 16:55, Andrew Cooper wrote:
>>> --- a/xen/include/asm-x86/processor.h
>>> +++ b/xen/include/asm-x86/processor.h
>>> @@ -411,7 +411,7 @@ static always_inline void __mwait(unsigned long eax, unsigned long ecx)
>>>   #define IOBMP_BYTES             8192
>>>   #define IOBMP_INVALID_OFFSET    0x8000
>>>   
>>> -struct __packed __cacheline_aligned tss_struct {
>>> +struct __packed tss_struct {
>>>       uint32_t :32;
>>>       uint64_t rsp0, rsp1, rsp2;
>>>       uint64_t :64;
>>> @@ -425,6 +425,7 @@ struct __packed __cacheline_aligned tss_struct {
>>>       /* Pads the TSS to be cacheline-aligned (total size is 0x80). */
>>>       uint8_t __cacheline_filler[24];
>>>   };
>>> +DECLARE_PER_CPU(struct tss_struct, init_tss);
>> Taking patch 1 this expands to
>>
>>      __DEFINE_PER_CPU(__section(".bss.percpu.page_aligned") \
>>                       __aligned(PAGE_SIZE), struct tss_struct, _init_tss);
>>
>> and then
>>
>>      __section(".bss.percpu.page_aligned") __aligned(PAGE_SIZE)
>>      __typeof__(struct tss_struct) per_cpu__init_tss;
>>
>> which is not what you want: You have an object of size
>> sizeof(struct tss_struct) which is PAGE_SIZE aligned. Afaict you
>> therefore still leak everything that follows in the same page.
> What data might this be?
>
> Every object put into this section is suitably aligned, so nothing will
> sit in the slack between the TSS and the end of the page.

And of course, I should have actually checked...

ffff82d08092e000 B per_cpu__init_tss
ffff82d08092e000 B __per_cpu_start
ffff82d08092e080 B per_cpu__cpupool
ffff82d08092e088 b per_cpu__continue_info

This needs fixing with suitable alignment in the linker script.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel