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

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

On 09.08.2019 16:59, Andrew Cooper wrote:
On 09/08/2019 13:32, Jan Beulich wrote:
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
TBD: Especially with how the previous patch now works I'm unconvinced of
      the utility of the linker script alignment check. It in particular
      doesn't check the property we're after in this patch, i.e. the fact
      that there's nothing else in the same page.

It should now probably be a BUILD_BUG_ON() checking sizeof(tss_page)
being exactly a page, given that there is also a compile time alignment

Will do.

NB: Sadly get_per_cpu_var() can't also be used on the "left" side of a

I don't understand what you are trying to imply with this.

It would be nice to be able to say

#define get_per_cpu_var(init_tss) get_per_cpu_var(init_tss_page.tss)

  That said, ...

--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -411,7 +411,7 @@ static always_inline void __mwait(unsign
  #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,11 @@ struct __packed __cacheline_aligned tss_
      /* Pads the TSS to be cacheline-aligned (total size is 0x80). */
      uint8_t __cacheline_filler[24];
+struct tss_page {
+    struct tss_struct __aligned(PAGE_SIZE) tss;
+DECLARE_PER_CPU(struct tss_page, init_tss_page);
+#define per_cpu__init_tss get_per_cpu_var(init_tss_page.tss)

... my first attempt started by renaming init_tss because the init part
is bogus.  I believe we inherited this nomenclature from Linux, but that
doesn't make it right.

I referred the renaming patch specifically to aid in backportability,
but given these changes to the type, leaving it alone isn't an option.

I'm not happy with introducing this diversion, because it means that all
users of per_cpu(init_tss) now have the wrong types in their hand from
the point of view of reading the code.

I'm still uncertain which is the least bad option between backporting
this version, and backporting the version which adjusts all users -
there aren't too many, and its definitely not the most awkward backport
we've ever had to do.

I could be persuaded into keeping this version for legacy trees, so long
as it doesn't propagate into master.  i.e. this patch drops the init_
prefix, and I'll rebase my renaming as a 3/2 again which gets committed
at around about the same time.

That way we retain a non-deceptive code in master.


I have no problem at all dropping the init_ here. It's not clear to me
though whether you saying "I'm not happy with introducing this diversion"
implies anything further that you'd want or even expect me to change.


Xen-devel mailing list



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