Skip to contentSkip to conversation

Patch 2

PATCH In review Andi Kleen 2 participants 2 messages May 2, 2018 x86

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 2/6 Patch 2

We also need to protect PTEs that are set to PROT_NONE against L1TF speculation attacks.

This is important inside guests, because L1TF speculation bypasses physical page remapping. While the VM has its own migitations preventing leaking data from other VMs into the guest, this would still risk leaking the wrong page inside the current guest.

This uses the same technique as Linus' swap entry patch:
while an entry is is in PROTNONE state we invert the complete PFN part part of it. This ensures that the the highest bit will point to non existing memory.

The invert is done by pte/pmd/pud_modify and pfn/pmd/pud_pte for PROTNONE and pte/pmd/pud_pfn undo it.

We assume that noone tries to touch the PFN part of a PTE without using these primitives.

This doesn't handle the case that MMIO is on the top of the CPU physical memory. If such an MMIO region was exposed by an unpriviledged driver for mmap it would be possible to attack some real memory. However this situation is all rather unlikely.

For 32bit non PAE we don't try inversion because there are really not enough bits to protect anything.

---
 arch/x86/include/asm/pgtable-2level.h | 27 +++++++++++++++++++
 arch/x86/include/asm/pgtable-3level.h |  2 ++
 arch/x86/include/asm/pgtable-invert.h | 45 +++++++++++++++++++++++++++++++
 arch/x86/include/asm/pgtable.h        | 50 ++++++++++++++++++++++++++---------
 arch/x86/include/asm/pgtable_64.h     |  2 ++
 5 files changed, 113 insertions(+), 13 deletions(-)
 create mode 100644 arch/x86/include/asm/pgtable-invert.h
arch/x86/include/asm/pgtable-2level.h +27 -0
index 685ffe8a0eaf..a1de6ae0c443 100644
@@ -95,4 +95,31 @@ static inline unsigned long pte_bitop(unsigned long value, unsigned int rightshi
 #define __pte_to_swp_entry(pte)		((swp_entry_t) { (pte).pte_low })
 #define __swp_entry_to_pte(x)		((pte_t) { .pte = (x).val })
 
+/* No inverted PFNs on 2 level page tables */
+
+static inline bool pte_pfn_inverted(pte_t pte)
+{
+	return false;
+}
+
+static inline bool pmd_pfn_inverted(pmd_t pmd)
+{
+	return false;
+}
+
+static inline bool pud_pfn_inverted(pud_t pud)
+{
+	return false;
+}
+
+static inline bool pgprot_pfn_inverted(pgprot_t prot)
+{
+	return false;
+}
+
+static inline u64 flip_protnone_guard(u64 oldval, u64 val, u64 mask)
+{
+	return val;
+}
+
 #endif /* _ASM_X86_PGTABLE_2LEVEL_H */
arch/x86/include/asm/pgtable-3level.h +2 -0
index f24df59c40b2..76ab26a99e6e 100644
@@ -295,4 +295,6 @@ static inline pte_t gup_get_pte(pte_t *ptep)
 	return pte;
 }
 
+#include <asm/pgtable-invert.h>
+
 #endif /* _ASM_X86_PGTABLE_3LEVEL_H */
arch/x86/include/asm/pgtable-invert.h new file +45 -0

45 lines not on this page · raw message

arch/x86/include/asm/pgtable.h +37 -13

98 lines not on this page · raw message

arch/x86/include/asm/pgtable_64.h +2 -0
index 593c3cf259dd..ea99272ab63e 100644
@@ -357,5 +357,7 @@ static inline bool gup_fast_permitted(unsigned long start, int nr_pages,
 	return true;
 }
 
+#include <asm/pgtable-invert.h>
+
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASM_X86_PGTABLE_64_H */
Signature · 1 line
2.15.0
#1 lore raw
Quoted 16 lines · speck for Andi Kleen

We also need to protect PTEs that are set to PROT_NONE against L1TF speculation attacks.

arch/x86/include/asm/pgtable-invert.h new file +14 -0
+
+static inline bool pte_pfn_inverted(pte_t pte)
+{
+	u64 val = pte_val(pte);
+	return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE;
+}
+
+static inline bool pmd_pfn_inverted(pmd_t pmd)
+{
+	u64 val = pmd_val(pmd);
+	return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE;
+}
+
+static inline bool pud_pfn_inverted(pud_t pud)

arch/x86/include/asm/pgtable-invert.h line 29

Quoted 10 lines
arch/x86/include/asm/pgtable-invert.h new file +10 -0
+{
+	u64 val = pud_val(pud);
+	return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE;
+}
+
+static inline bool pgprot_pfn_inverted(pgprot_t prot)
+{
+	u64 val = pgprot_val(prot);
+	return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE;
+}

Honestly, I don't hate this, but I do think it's a bit repetitious.

How about we just have a helper function that says "return ~0ul for the _PAGE_PROTNONE case, 0 otherwise". So something like this:

    unsigned long protnone_mask(unsigned long val)
    {
        return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE ? ~0ul : 0;
    }

Then you can write all those

        if (pte_pfn_inverted(pte))
                pfn = ~pfn;

things as

	pfn ^= protnone_mask(pte_val(pte));

and you don't need four different versions (you'll just use "pte_val()", "pmd_val()" etc).

arch/x86/include/asm/pgtable-invert.h line 41

Quoted 11 lines
arch/x86/include/asm/pgtable-invert.h new file +11 -0
+static inline u64 flip_protnone_guard(u64 oldval, u64 val, u64 mask)
+{
+	/*
+	 * When a PTE transitions from NONE to !NONE or vice-versa
+	 * invert the PFN part to stop speculation.
+	 * pte_pfn undoes this when needed.
+	 */
+	if ((oldval & _PAGE_PROTNONE) != (val & _PAGE_PROTNONE))
+		val = (val & ~mask) | (~val & mask);
+	return val;
+}

Ok, this is clever.

Linus