On 26/07/2019 15:38, Roger Pau Monné wrote:
> On Fri, Jul 26, 2019 at 02:52:40PM +0100, Andrew Cooper wrote:
>> The XPTI work restricted the visibility of most of memory, but missed a few
>> aspects when it came to the TSS.
>>
>> Given that the TSS is just an object in percpu data, the 4k mapping for it
>> created in setup_cpu_root_pgt() maps adjacent percpu data, making it all
>> leakable via Meltdown, even when XPTI is in use.
>>
>> Furthermore, no care is taken to check that the TSS doesn't cross a page
>> boundary. As it turns out, struct tss_struct is aligned on its size which
>> does prevent it straddling a page boundary, but this will cease to be true
>> once CET and Shadow Stack support is added to Xen.
>>
>> Move the TSS into the page aligned percpu area, so no adjacent data can be
>> leaked. Move the definition from setup.c to traps.c, which is a more
>> appropriate place for it to live.
>>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>> ---
>> CC: Jan Beulich <jbeul...@suse.com>
>> CC: Wei Liu <w...@xen.org>
>> CC: Roger Pau Monné <roger....@citrix.com>
>> ---
>> xen/arch/x86/setup.c | 2 --
>> xen/arch/x86/traps.c | 6 ++++++
>> xen/arch/x86/xen.lds.S | 2 ++
>> xen/include/asm-x86/processor.h | 4 ++--
>> 4 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index d2011910fa..1a2ffc4dc1 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -100,8 +100,6 @@ unsigned long __read_mostly xen_phys_start;
>>
>> unsigned long __read_mostly xen_virt_end;
>>
>> -DEFINE_PER_CPU(struct tss_struct, init_tss);
>> -
>> char __section(".bss.stack_aligned") __aligned(STACK_SIZE)
>> cpu0_stack[STACK_SIZE];
>>
>> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
>> index 38d12013db..e4b4587956 100644
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -108,6 +108,12 @@ idt_entry_t __section(".bss.page_aligned")
>> __aligned(PAGE_SIZE)
>> /* Pointer to the IDT of every CPU. */
>> idt_entry_t *idt_tables[NR_CPUS] __read_mostly;
>>
>> +/*
>> + * The TSS is smaller than a page, but we give it a full page to avoid
>> + * adjacent per-cpu data leaking via Meltdown when XPTI is in use.
>> + */
>> +DEFINE_PER_CPU_PAGE_ALIGNED(struct __aligned(PAGE_SIZE) tss_struct,
>> init_tss);
> Can't you bundle the __aligned attribute in
> DEFINE_PER_CPU_PAGE_ALIGNED itself?
>
> #define DEFINE_PER_CPU_PAGE_ALIGNED(type, name) \
> __DEFINE_PER_CPU(type __aligned(PAGE_SIZE), _##name, .page_aligned)
>
> I haven't tested this TBH.
I did. It doesn't compile, because the attribute follows the declaration.
Observe that the patch puts __aligned() between struct and tss_struct.
There is no way to do this inside DEFINE_PER_CPU_PAGE_ALIGNED(), because
we can't break the type apart to insert __aligned() in the middle.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
