[MODERATED] [PATCH v4 0/8] L1TFv4 0

[MODERATED] [PATCH v4 0/8] L1TFv4 0

[ak]

Here's the updated L1TF native OS patchkit to mitigate speculation
into unmapped PTEs.  This is stabilizing now.

This version has over v3:
- Cleanups/changes from code review
- Fix a bug added with v3 that broke the sysfs reporting

Andi Kleen (7):
  x86, l1tf: Increase 32bit PAE __PHYSICAL_PAGE_MASK
  x86, l1tf: Protect PROT_NONE PTEs against speculation
  x86, l1tf: Make sure the first page is always reserved
  x86, l1tf: Add sysfs reporting for l1tf
  x86, l1tf: Report if too much memory for L1TF workaround
  x86, l1tf: Limit swap file size to MAX_PA/2
  mm, l1tf: Disallow non privileged high MMIO PROT_NONE mappings

Linus Torvalds (1):
  x86, l1tf: Protect swap entries against L1TF

 arch/x86/include/asm/cpufeature.h     |  5 ++++
 arch/x86/include/asm/cpufeatures.h    |  2 ++
 arch/x86/include/asm/page_32_types.h  |  9 +++++--
 arch/x86/include/asm/pgtable-2level.h | 17 ++++++++++++
 arch/x86/include/asm/pgtable-3level.h |  2 ++
 arch/x86/include/asm/pgtable-invert.h | 32 +++++++++++++++++++++++
 arch/x86/include/asm/pgtable.h        | 48 ++++++++++++++++++++++++----------
 arch/x86/include/asm/pgtable_64.h     | 38 +++++++++++++++++++--------
 arch/x86/include/asm/processor.h      |  5 ++++
 arch/x86/kernel/cpu/bugs.c            | 11 ++++++++
 arch/x86/kernel/cpu/common.c          | 15 ++++++++++-
 arch/x86/kernel/setup.c               | 28 +++++++++++++++++++-
 arch/x86/mm/init.c                    | 15 +++++++++++
 arch/x86/mm/mmap.c                    | 19 ++++++++++++++
 drivers/base/cpu.c                    |  8 ++++++
 include/asm-generic/pgtable.h         | 12 +++++++++
 include/linux/cpu.h                   |  2 ++
 include/linux/swapfile.h              |  2 ++
 mm/memory.c                           | 37 +++++++++++++++++++-------
 mm/mprotect.c                         | 49 +++++++++++++++++++++++++++++++++++
 mm/swapfile.c                         | 44 +++++++++++++++++++------------
 21 files changed, 346 insertions(+), 54 deletions(-)
 create mode 100644 arch/x86/include/asm/pgtable-invert.h

-- 
2.14.3

[MODERATED] [PATCH v4 1/8] L1TFv4 1

[ak]

On 32bit PAE the max PTE mask is currently set to 44 bit because that is
the limit imposed by 32bit unsigned long PFNs in the VMs.

The L1TF PROT_NONE protection code uses the PTE masks to determine
what bits to invert to make sure the higher bits are set for unmapped
entries to prevent L1TF speculation attacks against EPT inside guests.

But our inverted mask has to match the host, and the host is likely
64bit and may use more than 43 bits of memory. We want to set
all possible bits to be safe here.

So increase the mask on 32bit PAE to 52 to match 64bit. The real
limit is still 44 bits but outside the inverted PTEs these
higher bits are set, so a bigger masks don't cause any problems.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/page_32_types.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/page_32_types.h b/arch/x86/include/asm/page_32_types.h
index aa30c3241ea7..0d5c739eebd7 100644
--- a/arch/x86/include/asm/page_32_types.h
+++ b/arch/x86/include/asm/page_32_types.h
@@ -29,8 +29,13 @@
 #define N_EXCEPTION_STACKS 1
 
 #ifdef CONFIG_X86_PAE
-/* 44=32+12, the limit we can fit into an unsigned long pfn */
-#define __PHYSICAL_MASK_SHIFT	44
+/*
+ * This is beyond the 44 bit limit imposed by the 32bit long pfns,
+ * but we need the full mask to make sure inverted PROT_NONE
+ * entries have all the host bits set in a guest.
+ * The real limit is still 44 bits.
+ */
+#define __PHYSICAL_MASK_SHIFT	52
 #define __VIRTUAL_MASK_SHIFT	32
 
 #else  /* !CONFIG_X86_PAE */
-- 
2.14.3

Re: [PATCH v4 1/8] L1TFv4 1

[Thomas Gleixner]

On Wed, 9 May 2018, speck for ak_at_linux.intel.com wrote:
> From: Andi Kleen <ak@linux.intel.com>
> Subject:  x86, l1tf: Increase 32bit PAE __PHYSICAL_PAGE_MASK
> 
> On 32bit PAE the max PTE mask is currently set to 44 bit because that is
> the limit imposed by 32bit unsigned long PFNs in the VMs.
> 
> The L1TF PROT_NONE protection code uses the PTE masks to determine
> what bits to invert to make sure the higher bits are set for unmapped
> entries to prevent L1TF speculation attacks against EPT inside guests.
> 
> But our inverted mask has to match the host, and the host is likely
> 64bit and may use more than 43 bits of memory. We want to set
> all possible bits to be safe here.
> 
> So increase the mask on 32bit PAE to 52 to match 64bit. The real
> limit is still 44 bits but outside the inverted PTEs these
> higher bits are set, so a bigger masks don't cause any problems.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  arch/x86/include/asm/page_32_types.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/page_32_types.h b/arch/x86/include/asm/page_32_types.h
> index aa30c3241ea7..0d5c739eebd7 100644
> --- a/arch/x86/include/asm/page_32_types.h
> +++ b/arch/x86/include/asm/page_32_types.h
> @@ -29,8 +29,13 @@
>  #define N_EXCEPTION_STACKS 1
>  
>  #ifdef CONFIG_X86_PAE
> -/* 44=32+12, the limit we can fit into an unsigned long pfn */
> -#define __PHYSICAL_MASK_SHIFT	44
> +/*
> + * This is beyond the 44 bit limit imposed by the 32bit long pfns,
> + * but we need the full mask to make sure inverted PROT_NONE
> + * entries have all the host bits set in a guest.
> + * The real limit is still 44 bits.
> + */
> +#define __PHYSICAL_MASK_SHIFT	52
>  #define __VIRTUAL_MASK_SHIFT	32

Are you really sure that this does not have unwanted side effects in any
(indirect) user of __PHYSICAL_MASK_SHIFT ? If yes, please indicate so in
the changelog.

Thanks,

	tglx

[MODERATED] Re: [PATCH v4 1/8] L1TFv4 1

[Andi Kleen]

> > -/* 44=32+12, the limit we can fit into an unsigned long pfn */
> > -#define __PHYSICAL_MASK_SHIFT	44
> > +/*
> > + * This is beyond the 44 bit limit imposed by the 32bit long pfns,
> > + * but we need the full mask to make sure inverted PROT_NONE
> > + * entries have all the host bits set in a guest.
> > + * The real limit is still 44 bits.
> > + */
> > +#define __PHYSICAL_MASK_SHIFT	52
> >  #define __VIRTUAL_MASK_SHIFT	32
> 
> Are you really sure that this does not have unwanted side effects in any
> (indirect) user of __PHYSICAL_MASK_SHIFT ? If yes, please indicate so in
> the changelog.

Yes I did an audit. Will indicate.

It's safe because these bits can never be set to non 0 because all PTEs
are created from unsigned long PFNs.

-Andi

[MODERATED] [PATCH v4 2/8] L1TFv4 4

[ak]

With L1 terminal fault the CPU speculates into unmapped PTEs, and
resulting side effects allow to read the memory the PTE is pointing
too, if its values are still in the L1 cache.

For swapped out pages Linux uses unmapped PTEs and stores a swap entry
into them.

We need to make sure the swap entry is not pointing to valid memory,
which requires setting higher bits (between bit 36 and bit 45) that
are inside the CPUs physical address space, but outside any real
memory.

To do this we invert the offset to make sure the higher bits are always
set, as long as the swap file is not too big.

Here's a patch that switches the order of "type" and
"offset" in the x86-64 encoding, in addition to doing the binary 'not' on
the offset.

That means that now the offset is bits 9-58 in the page table, and that
the offset is in the bits that hardware generally doesn't care about.

That, in turn, means that if you have a desktop chip with only 40 bits of
physical addressing, now that the offset starts at bit 9, you still have
to have 30 bits of offset actually *in use* until bit 39 ends up being
clear.

So that's 4 terabyte of swap space (because the offset is counted in
pages, so 30 bits of offset is 42 bits of actual coverage). With bigger
physical addressing, that obviously grows further, until you hit the limit
of the offset (at 50 bits of offset - 62 bits of actual swap file
coverage).

Note there is no workaround for 32bit !PAE, or on systems which
have more than MAX_PA/2 memory. The later case is very unlikely
to happen on real systems.

[updated description and minor tweaks by AK]

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Tested-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 arch/x86/include/asm/pgtable_64.h | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 877bc27718ae..593c3cf259dd 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -273,7 +273,7 @@ static inline int pgd_large(pgd_t pgd) { return 0; }
  *
  * |     ...            | 11| 10|  9|8|7|6|5| 4| 3|2| 1|0| <- bit number
  * |     ...            |SW3|SW2|SW1|G|L|D|A|CD|WT|U| W|P| <- bit names
- * | OFFSET (14->63) | TYPE (9-13)  |0|0|X|X| X| X|X|SD|0| <- swp entry
+ * | TYPE (59-63) | ~OFFSET (9-58)  |0|0|X|X| X| X|X|SD|0| <- swp entry
  *
  * G (8) is aliased and used as a PROT_NONE indicator for
  * !present ptes.  We need to start storing swap entries above
@@ -286,20 +286,34 @@ static inline int pgd_large(pgd_t pgd) { return 0; }
  *
  * Bit 7 in swp entry should be 0 because pmd_present checks not only P,
  * but also L and G.
+ *
+ * The offset is inverted by a binary not operation to make the high
+ * physical bits set.
  */
-#define SWP_TYPE_FIRST_BIT (_PAGE_BIT_PROTNONE + 1)
-#define SWP_TYPE_BITS 5
-/* Place the offset above the type: */
-#define SWP_OFFSET_FIRST_BIT (SWP_TYPE_FIRST_BIT + SWP_TYPE_BITS)
+#define SWP_TYPE_BITS		5
+
+#define SWP_OFFSET_FIRST_BIT	(_PAGE_BIT_PROTNONE + 1)
+
+/* We always extract/encode the offset by shifting it all the way up, and then down again */
+#define SWP_OFFSET_SHIFT	(SWP_OFFSET_FIRST_BIT+SWP_TYPE_BITS)
 
 #define MAX_SWAPFILES_CHECK() BUILD_BUG_ON(MAX_SWAPFILES_SHIFT > SWP_TYPE_BITS)
 
-#define __swp_type(x)			(((x).val >> (SWP_TYPE_FIRST_BIT)) \
-					 & ((1U << SWP_TYPE_BITS) - 1))
-#define __swp_offset(x)			((x).val >> SWP_OFFSET_FIRST_BIT)
-#define __swp_entry(type, offset)	((swp_entry_t) { \
-					 ((type) << (SWP_TYPE_FIRST_BIT)) \
-					 | ((offset) << SWP_OFFSET_FIRST_BIT) })
+/* Extract the high bits for type */
+#define __swp_type(x) ((x).val >> (64 - SWP_TYPE_BITS))
+
+/* Shift up (to get rid of type), then down to get value */
+#define __swp_offset(x) (~(x).val << SWP_TYPE_BITS >> SWP_OFFSET_SHIFT)
+
+/*
+ * Shift the offset up "too far" by TYPE bits, then down again
+ * The offset is inverted by a binary not operation to make the high
+ * physical bits set.
+ */
+#define __swp_entry(type, offset) ((swp_entry_t) { \
+	(~(unsigned long)(offset) << SWP_OFFSET_SHIFT >> SWP_TYPE_BITS) \
+	| ((unsigned long)(type) << (64-SWP_TYPE_BITS)) })
+
 #define __pte_to_swp_entry(pte)		((swp_entry_t) { pte_val((pte)) })
 #define __pmd_to_swp_entry(pmd)		((swp_entry_t) { pmd_val((pmd)) })
 #define __swp_entry_to_pte(x)		((pte_t) { .pte = (x).val })
-- 
2.14.3

[MODERATED] [PATCH v4 3/8] L1TFv4 6

[ak]

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.

Q: Why does the guest need to be protected when the
HyperVisor already has L1TF mitigations?
A: Here's an example:
You have physical pages 1 2. They get mapped into a guest as
GPA 1 -> PA 2
GPA 2 -> PA 1
through EPT.

The L1TF speculation ignores the EPT remapping.

Now the guest kernel maps GPA 1 to process A and GPA 2 to process B,
and they belong to different users and should be isolated.

A sets the GPA 1 PA 2 PTE to PROT_NONE to bypass the EPT remapping
and gets read access to the underlying physical page. Which
in this case points to PA 2, so it can read process B's data,
if it happened to be in L1.

So we broke isolation inside the guest.

There's nothing the hypervisor can do about this. This
mitigation has to be done in the guest.

v2: Use new helper to generate XOR mask to invert (Linus)
v3: Use inline helper for protnone mask checking
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 arch/x86/include/asm/pgtable-2level.h | 17 ++++++++++++++
 arch/x86/include/asm/pgtable-3level.h |  2 ++
 arch/x86/include/asm/pgtable-invert.h | 32 +++++++++++++++++++++++++
 arch/x86/include/asm/pgtable.h        | 44 ++++++++++++++++++++++++-----------
 arch/x86/include/asm/pgtable_64.h     |  2 ++
 5 files changed, 84 insertions(+), 13 deletions(-)
 create mode 100644 arch/x86/include/asm/pgtable-invert.h

diff --git a/arch/x86/include/asm/pgtable-2level.h b/arch/x86/include/asm/pgtable-2level.h
index 685ffe8a0eaf..60d0f9015317 100644
--- a/arch/x86/include/asm/pgtable-2level.h
+++ b/arch/x86/include/asm/pgtable-2level.h
@@ -95,4 +95,21 @@ 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 u64 protnone_mask(u64 val)
+{
+	return 0;
+}
+
+static inline u64 flip_protnone_guard(u64 oldval, u64 val, u64 mask)
+{
+	return val;
+}
+
+static inline bool __pte_needs_invert(u64 val)
+{
+	return false;
+}
+
 #endif /* _ASM_X86_PGTABLE_2LEVEL_H */
diff --git a/arch/x86/include/asm/pgtable-3level.h b/arch/x86/include/asm/pgtable-3level.h
index f24df59c40b2..76ab26a99e6e 100644
--- a/arch/x86/include/asm/pgtable-3level.h
+++ b/arch/x86/include/asm/pgtable-3level.h
@@ -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 */
diff --git a/arch/x86/include/asm/pgtable-invert.h b/arch/x86/include/asm/pgtable-invert.h
new file mode 100644
index 000000000000..c740606b0c02
--- /dev/null
+++ b/arch/x86/include/asm/pgtable-invert.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_PGTABLE_INVERT_H
+#define _ASM_PGTABLE_INVERT_H 1
+
+#ifndef __ASSEMBLY__
+
+static inline bool __pte_needs_invert(u64 val)
+{
+	return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE;
+}
+
+/* Get a mask to xor with the page table entry to get the correct pfn. */
+static inline u64 protnone_mask(u64 val)
+{
+	return __pte_needs_invert(val) ?  ~0ull : 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;
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 5f49b4ff0c24..f811e3257e87 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -185,19 +185,29 @@ static inline int pte_special(pte_t pte)
 	return pte_flags(pte) & _PAGE_SPECIAL;
 }
 
+/* Entries that were set to PROT_NONE are inverted */
+
+static inline u64 protnone_mask(u64 val);
+
 static inline unsigned long pte_pfn(pte_t pte)
 {
-	return (pte_val(pte) & PTE_PFN_MASK) >> PAGE_SHIFT;
+	unsigned long pfn = pte_val(pte);
+	pfn ^= protnone_mask(pfn);
+	return (pfn & PTE_PFN_MASK) >> PAGE_SHIFT;
 }
 
 static inline unsigned long pmd_pfn(pmd_t pmd)
 {
-	return (pmd_val(pmd) & pmd_pfn_mask(pmd)) >> PAGE_SHIFT;
+	unsigned long pfn = pmd_val(pmd);
+	pfn ^= protnone_mask(pfn);
+	return (pfn & pmd_pfn_mask(pmd)) >> PAGE_SHIFT;
 }
 
 static inline unsigned long pud_pfn(pud_t pud)
 {
-	return (pud_val(pud) & pud_pfn_mask(pud)) >> PAGE_SHIFT;
+	unsigned long pfn = pud_val(pud);
+	pfn ^= protnone_mask(pfn);
+	return (pfn & pud_pfn_mask(pud)) >> PAGE_SHIFT;
 }
 
 static inline unsigned long p4d_pfn(p4d_t p4d)
@@ -545,25 +555,33 @@ static inline pgprotval_t check_pgprot(pgprot_t pgprot)
 
 static inline pte_t pfn_pte(unsigned long page_nr, pgprot_t pgprot)
 {
-	return __pte(((phys_addr_t)page_nr << PAGE_SHIFT) |
-		     check_pgprot(pgprot));
+	phys_addr_t pfn = page_nr << PAGE_SHIFT;
+	pfn ^= protnone_mask(pgprot_val(pgprot));
+	pfn &= PTE_PFN_MASK;
+	return __pte(pfn | check_pgprot(pgprot));
 }
 
 static inline pmd_t pfn_pmd(unsigned long page_nr, pgprot_t pgprot)
 {
-	return __pmd(((phys_addr_t)page_nr << PAGE_SHIFT) |
-		     check_pgprot(pgprot));
+	phys_addr_t pfn = page_nr << PAGE_SHIFT;
+	pfn ^= protnone_mask(pgprot_val(pgprot));
+	pfn &= PHYSICAL_PMD_PAGE_MASK;
+	return __pmd(pfn | check_pgprot(pgprot));
 }
 
 static inline pud_t pfn_pud(unsigned long page_nr, pgprot_t pgprot)
 {
-	return __pud(((phys_addr_t)page_nr << PAGE_SHIFT) |
-		     check_pgprot(pgprot));
+	phys_addr_t pfn = page_nr << PAGE_SHIFT;
+	pfn ^= protnone_mask(pgprot_val(pgprot));
+	pfn &= PHYSICAL_PUD_PAGE_MASK;
+	return __pud(pfn | check_pgprot(pgprot));
 }
 
+static inline u64 flip_protnone_guard(u64 oldval, u64 val, u64 mask);
+
 static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 {
-	pteval_t val = pte_val(pte);
+	pteval_t val = pte_val(pte), oldval = val;
 
 	/*
 	 * Chop off the NX bit (if present), and add the NX portion of
@@ -571,17 +589,17 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 	 */
 	val &= _PAGE_CHG_MASK;
 	val |= check_pgprot(newprot) & ~_PAGE_CHG_MASK;
-
+	val = flip_protnone_guard(oldval, val, PTE_PFN_MASK);
 	return __pte(val);
 }
 
 static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
 {
-	pmdval_t val = pmd_val(pmd);
+	pmdval_t val = pmd_val(pmd), oldval = val;
 
 	val &= _HPAGE_CHG_MASK;
 	val |= check_pgprot(newprot) & ~_HPAGE_CHG_MASK;
-
+	val = flip_protnone_guard(oldval, val, PHYSICAL_PMD_PAGE_MASK);
 	return __pmd(val);
 }
 
diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 593c3cf259dd..ea99272ab63e 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -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 */
-- 
2.14.3

Re: [PATCH v4 3/8] L1TFv4 6

[Thomas Gleixner]

On Wed, 9 May 2018, speck for ak_at_linux.intel.com wrote:
> diff --git a/arch/x86/include/asm/pgtable-invert.h b/arch/x86/include/asm/pgtable-invert.h
> new file mode 100644
> index 000000000000..c740606b0c02
> --- /dev/null
> +++ b/arch/x86/include/asm/pgtable-invert.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_PGTABLE_INVERT_H
> +#define _ASM_PGTABLE_INVERT_H 1
> +
> +#ifndef __ASSEMBLY__
> +
> +static inline bool __pte_needs_invert(u64 val)
> +{
> +	return (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE;
> +}
> +
> +/* Get a mask to xor with the page table entry to get the correct pfn. */
> +static inline u64 protnone_mask(u64 val)
> +{
> +	return __pte_needs_invert(val) ?  ~0ull : 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))

I might be missing something, but. pte_pfn() uses protnone_mask() which in
turn uses __pte_needs_invert(). __pte_needs_invert() checks for

     (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE

so the above condition is not matching the counterpart. So shouldn't that
be:

	if (__pte_needs_invert(oldval) != __pte_needs_invert(val))

Hmm?

> +		val = (val & ~mask) | (~val & mask);

Thanks,

	tglx

[MODERATED] Re: [PATCH v4 3/8] L1TFv4 6

[Andi Kleen]

> > +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))
> 
> I might be missing something, but. pte_pfn() uses protnone_mask() which in
> turn uses __pte_needs_invert(). __pte_needs_invert() checks for
> 
>      (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE
> 
> so the above condition is not matching the counterpart. So shouldn't that
> be:
> 
> 	if (__pte_needs_invert(oldval) != __pte_needs_invert(val))
> 
> Hmm?

_PAGE_PROTNONE aliases with global. For the general case you
have to check P to avoid mishandling kernel ptes.

I believe it's safe in that case because noone will change global
through pte_modify.

But can use the inlines too. It likely will generate a few more
instructions, but I doubt we need any micro optimization here.

-Andi

Re: [PATCH v4 3/8] L1TFv4 6

[Thomas Gleixner]

On Thu, 10 May 2018, speck for Andi Kleen wrote:
> > > +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))
> > 
> > I might be missing something, but. pte_pfn() uses protnone_mask() which in
> > turn uses __pte_needs_invert(). __pte_needs_invert() checks for
> > 
> >      (val & (_PAGE_PRESENT|_PAGE_PROTNONE)) == _PAGE_PROTNONE
> > 
> > so the above condition is not matching the counterpart. So shouldn't that
> > be:
> > 
> > 	if (__pte_needs_invert(oldval) != __pte_needs_invert(val))
> > 
> > Hmm?
> 
> _PAGE_PROTNONE aliases with global. For the general case you
> have to check P to avoid mishandling kernel ptes.

Right.

> I believe it's safe in that case because noone will change global
> through pte_modify.
> 
> But can use the inlines too. It likely will generate a few more
> instructions, but I doubt we need any micro optimization here.

Even if it's just for consistency sake for now, it makes it simpler to
understand 6 month from now and it will prevent the odd user of
pte_modify() which clears the global bit to run into a hard to debug
problem.

Thanks,

	tglx

[MODERATED] [PATCH v4 4/8] L1TFv4 7

[ak]

The L1TF workaround doesn't make any attempt to mitigate speculate
accesses to the first physical page for zeroed PTEs. Normally
it only contains some data from the early real mode BIOS.

I couldn't convince myself we always reserve the first page in
all configurations, so add an extra reservation call to
make sure it is really reserved. In most configurations (e.g.
with the standard reservations) it's likely a nop.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/setup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 6285697b6e56..fadbd41094d2 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -817,6 +817,9 @@ void __init setup_arch(char **cmdline_p)
 	memblock_reserve(__pa_symbol(_text),
 			 (unsigned long)__bss_stop - (unsigned long)_text);
 
+	/* Make sure page 0 is always reserved */
+	memblock_reserve(0, PAGE_SIZE);
+
 	early_reserve_initrd();
 
 	/*
-- 
2.14.3

[MODERATED] [PATCH v4 5/8] L1TFv4 8

[ak]

L1TF core kernel workarounds are cheap and normally always enabled,
However we still want to report in sysfs if the system is vulnerable
or mitigated. Add the necessary checks.

- We use the same checks as Meltdown to determine if the system is
vulnerable. This excludes some Atom CPUs which don't have this
problem.
- We check for the (very unlikely) memory > MAX_PA/2 case
- We check for 32bit non PAE and warn

Note this patch will likely conflict with some other workaround patches
floating around, but should be straight forward to fix.

v2: Use positive instead of negative flag for WA. Fix override
reporting.
v3: Fix L1TF_WA flag settting
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h |  2 ++
 arch/x86/kernel/cpu/bugs.c         | 11 +++++++++++
 arch/x86/kernel/cpu/common.c       | 15 ++++++++++++++-
 drivers/base/cpu.c                 |  8 ++++++++
 include/linux/cpu.h                |  2 ++
 5 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index d554c11e01ff..f1bfe8a37b84 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -214,6 +214,7 @@
 
 #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
 #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
+#define X86_FEATURE_L1TF_WA		( 7*32+23) /* "" L1TF workaround used */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
@@ -362,5 +363,6 @@
 #define X86_BUG_CPU_MELTDOWN		X86_BUG(14) /* CPU is affected by meltdown attack and needs kernel page table isolation */
 #define X86_BUG_SPECTRE_V1		X86_BUG(15) /* CPU is affected by Spectre variant 1 attack with conditional branches */
 #define X86_BUG_SPECTRE_V2		X86_BUG(16) /* CPU is affected by Spectre variant 2 attack with indirect branches */
+#define X86_BUG_L1TF			X86_BUG(17) /* CPU is affected by L1 Terminal Fault */
 
 #endif /* _ASM_X86_CPUFEATURES_H */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index bfca937bdcc3..e1f67b7c5217 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -340,4 +340,15 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, c
 		       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
 		       spectre_v2_module_string());
 }
+
+ssize_t cpu_show_l1tf(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	if (!boot_cpu_has_bug(X86_BUG_L1TF))
+		return sprintf(buf, "Not affected\n");
+
+	if (boot_cpu_has(X86_FEATURE_L1TF_WA))
+		return sprintf(buf, "Mitigated\n");
+
+	return sprintf(buf, "Mitigation Unavailable\n");
+}
 #endif
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8a5b185735e1..8bb14ccb2f4b 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -940,6 +940,15 @@ static bool __init cpu_vulnerable_to_meltdown(struct cpuinfo_x86 *c)
 	return true;
 }
 
+static bool __init l1tf_wa_possible(void)
+{
+#if CONFIG_PGTABLE_LEVELS == 2
+	pr_warn("Kernel not compiled for PAE. No workaround for L1TF\n");
+	return false;
+#endif
+	return true;
+}
+
 /*
  * Do minimum CPU detection early.
  * Fields really needed: vendor, cpuid_level, family, model, mask,
@@ -989,8 +998,12 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 	setup_force_cpu_cap(X86_FEATURE_ALWAYS);
 
 	if (!x86_match_cpu(cpu_no_speculation)) {
-		if (cpu_vulnerable_to_meltdown(c))
+		if (cpu_vulnerable_to_meltdown(c)) {
 			setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
+			setup_force_cpu_bug(X86_BUG_L1TF);
+			if (l1tf_wa_possible())
+				setup_force_cpu_cap(X86_FEATURE_L1TF_WA);
+		}
 		setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
 		setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
 	}
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 2da998baa75c..ed7b8591d461 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -534,14 +534,22 @@ ssize_t __weak cpu_show_spectre_v2(struct device *dev,
 	return sprintf(buf, "Not affected\n");
 }
 
+ssize_t __weak cpu_show_l1tf(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "Not affected\n");
+}
+
 static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL);
 static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL);
 static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);
+static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL);
 
 static struct attribute *cpu_root_vulnerabilities_attrs[] = {
 	&dev_attr_meltdown.attr,
 	&dev_attr_spectre_v1.attr,
 	&dev_attr_spectre_v2.attr,
+	&dev_attr_l1tf.attr,
 	NULL
 };
 
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 7b01bc11c692..75c430046ca0 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -53,6 +53,8 @@ extern ssize_t cpu_show_spectre_v1(struct device *dev,
 				   struct device_attribute *attr, char *buf);
 extern ssize_t cpu_show_spectre_v2(struct device *dev,
 				   struct device_attribute *attr, char *buf);
+extern ssize_t cpu_show_l1tf(struct device *dev,
+				   struct device_attribute *attr, char *buf);
 
 extern __printf(4, 5)
 struct device *cpu_device_create(struct device *parent, void *drvdata,
-- 
2.14.3

Re: [PATCH v4 5/8] L1TFv4 8

[Thomas Gleixner]

> +ssize_t cpu_show_l1tf(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	if (!boot_cpu_has_bug(X86_BUG_L1TF))
> +		return sprintf(buf, "Not affected\n");
> +
> +	if (boot_cpu_has(X86_FEATURE_L1TF_WA))
> +		return sprintf(buf, "Mitigated\n");
> +
> +	return sprintf(buf, "Mitigation Unavailable\n");

That's 'Vulnerable' for all other issues, so please make it consistent.

> +static bool __init l1tf_wa_possible(void)
> +{
> +#if CONFIG_PGTABLE_LEVELS == 2
> +	pr_warn("Kernel not compiled for PAE. No workaround for L1TF\n");
> +	return false;
> +#endif
> +	return true;
> +}
> +
>  /*
>   * Do minimum CPU detection early.
>   * Fields really needed: vendor, cpuid_level, family, model, mask,
> @@ -989,8 +998,12 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
>  	setup_force_cpu_cap(X86_FEATURE_ALWAYS);
>  
>  	if (!x86_match_cpu(cpu_no_speculation)) {
> -		if (cpu_vulnerable_to_meltdown(c))
> +		if (cpu_vulnerable_to_meltdown(c)) {
>  			setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> +			setup_force_cpu_bug(X86_BUG_L1TF);
> +			if (l1tf_wa_possible())
> +				setup_force_cpu_cap(X86_FEATURE_L1TF_WA);

That extra indirection is pointless. 

static void __init l1tf_init_workaround(void)
{
#if CONFIG_PGTABLE_LEVELS == 2
	pr_warn("Kernel not compiled for PAE. No workaround for L1TF\n");
#else
	setup_force_cpu_cap(X86_FEATURE_L1TF_WA);
#endif
}

Hmm?

> +ssize_t __weak cpu_show_l1tf(struct device *dev,
> +				   struct device_attribute *attr, char *buf)

Please align the arguments in the second row in the same way as all other
weak functions do. 

Thanks,

	tglx

[MODERATED] [PATCH v4 6/8] L1TFv4 3

[ak]

If the system has more than MAX_PA/2 physical memory the
invert page workarounds don't protect the system against
the L1TF attack anymore, because an inverted physical address
will point to valid memory.

We cannot do much here, after all users want to use the
memory, but at least print a warning and report the system as
vulnerable in sysfs

Note this is all extremely unlikely to happen on a real machine
because they typically have far more MAX_PA than DIMM slots

Some VMs also report fairly small PAs to guest, e.g. only 36bits.
In this case the threshold will be lower, but applies only
to the maximum guest size.

Since this needs to clear a feature bit that has been forced
earlier add a special "unforce" macro that supports this.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/cpufeature.h |  5 +++++
 arch/x86/kernel/setup.c           | 25 ++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index b27da9602a6d..f78bfd2464c1 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -138,6 +138,11 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
 	set_bit(bit, (unsigned long *)cpu_caps_set);	\
 } while (0)
 
+#define setup_unforce_cpu_cap(bit) do { \
+	clear_cpu_cap(&boot_cpu_data, bit);	\
+	clear_bit(bit, (unsigned long *)cpu_caps_set);	\
+} while (0)
+
 #define setup_force_cpu_bug(bit) setup_force_cpu_cap(bit)
 
 /*
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index fadbd41094d2..b49fcb3e3a97 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -779,7 +779,28 @@ static void __init trim_low_memory_range(void)
 {
 	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
 }
-	
+
+static __init void check_maxpa_memory(void)
+{
+	u64 len;
+
+	if (!boot_cpu_has(X86_BUG_L1TF))
+		return;
+
+	len = BIT_ULL(boot_cpu_data.x86_phys_bits - 1) - 1;
+
+	/*
+	 * This is extremely unlikely to happen because systems near always have far
+	 * more MAX_PA than DIMM slots.
+	 */
+	if (e820__mapped_any(len, ULLONG_MAX - len,
+				     E820_TYPE_RAM)) {
+		pr_warn("System has more than MAX_PA/2 memory. Disabled L1TF workaround\n");
+		/* Was forced earlier, so now unforce it. */
+		setup_unforce_cpu_cap(X86_FEATURE_L1TF_WA);
+	}
+}
+
 /*
  * Dump out kernel offset information on panic.
  */
@@ -1016,6 +1037,8 @@ void __init setup_arch(char **cmdline_p)
 	insert_resource(&iomem_resource, &data_resource);
 	insert_resource(&iomem_resource, &bss_resource);
 
+	check_maxpa_memory();
+
 	e820_add_kernel_range();
 	trim_bios_range();
 #ifdef CONFIG_X86_32
-- 
2.14.3

Re: [PATCH v4 6/8] L1TFv4 3

[Thomas Gleixner]

On Wed, 9 May 2018, speck for ak_at_linux.intel.com wrote:
> Since this needs to clear a feature bit that has been forced
> earlier add a special "unforce" macro that supports this.

There is no reason to add yet another macro to the cpu feature maze. The
right thing to do is to clear the forced bit from setup_clear_cpu_cap().

> +static __init void check_maxpa_memory(void)
> +{
> +	u64 len;
> +
> +	if (!boot_cpu_has(X86_BUG_L1TF))
> +		return;
> +
> +	len = BIT_ULL(boot_cpu_data.x86_phys_bits - 1) - 1;

Please name that variable max_pa

> +
> +	/*
> +	 * This is extremely unlikely to happen because systems near always have far

s/near/almost/

> +	 * more MAX_PA than DIMM slots.

That makes no sense, What has MAX_PA to do with the number of DIMM slots?

MAX_PA is the maximum physical address at least that what it has been used
for so far. So obviously MAX_PA is always larger than the number of DIMM
slots....

> +	 */
> +	if (e820__mapped_any(len, ULLONG_MAX - len,
> +				     E820_TYPE_RAM)) {

This line break is required becasue ... the line has been copied from an
indented block ...

Thanks,

	tglx

[MODERATED] Re: [PATCH v4 6/8] L1TFv4 3

[Andi Kleen]

> > +
> > +	/*
> > +	 * This is extremely unlikely to happen because systems near always have far
> 
> s/near/almost/

There are some very rare systems where it may, mainly SGI UV.

-Andi

Re: [PATCH v4 6/8] L1TFv4 3

[Thomas Gleixner]

On Thu, 10 May 2018, speck for Andi Kleen wrote:
> > > +
> > > +	/*
> > > +	 * This is extremely unlikely to happen because systems near always have far
> > 
> > s/near/almost/
> 
> There are some very rare systems where it may, mainly SGI UV.

Sure, but that does not change the fact that 'near always' should be
'almost always' :)

Maybe rephrase it to:

    This is extremely unlikely to happen because almost all systems have.....

Thanks,

	tglx

[MODERATED] [PATCH v4 7/8] L1TFv4 2

[ak]

For the L1TF workaround we want to limit the swap file size to below
MAX_PA/2, so that the higher bits of the swap offset inverted never
point to valid memory.

Add a way for the architecture to override the swap file
size check in swapfile.c and add a x86 specific max swapfile check
function that enforces that limit.

The check is only enabled if the CPU is vulnerable to L1TF.

In VMs with 42bit MAX_PA the typical limit is 2TB now,
on a native system with 46bit PA it is 32TB. The limit
is only per individual swap file, so it's always possible
to exceed these limits with multiple swap files or
partitions.

v2: Use new helper for maxpa_mask computation.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/processor.h |  5 +++++
 arch/x86/mm/init.c               | 15 ++++++++++++++
 include/linux/swapfile.h         |  2 ++
 mm/swapfile.c                    | 44 +++++++++++++++++++++++++---------------
 4 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 21a114914ba4..2bd676e450cf 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -181,6 +181,11 @@ extern const struct seq_operations cpuinfo_op;
 
 extern void cpu_detect(struct cpuinfo_x86 *c);
 
+static inline u64 maxpa_pfn_bit(int offset)
+{
+	return BIT_ULL(boot_cpu_data.x86_phys_bits - offset - PAGE_SHIFT);
+}
+
 extern void early_cpu_init(void);
 extern void identify_boot_cpu(void);
 extern void identify_secondary_cpu(struct cpuinfo_x86 *);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index fec82b577c18..b4078eb05ca0 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -4,6 +4,8 @@
 #include <linux/swap.h>
 #include <linux/memblock.h>
 #include <linux/bootmem.h>	/* for max_low_pfn */
+#include <linux/swapfile.h>
+#include <linux/swapops.h>
 
 #include <asm/set_memory.h>
 #include <asm/e820/api.h>
@@ -878,3 +880,16 @@ void update_cache_mode_entry(unsigned entry, enum page_cache_mode cache)
 	__cachemode2pte_tbl[cache] = __cm_idx2pte(entry);
 	__pte2cachemode_tbl[entry] = cache;
 }
+
+unsigned long max_swapfile_size(void)
+{
+	unsigned long pages;
+
+	pages = generic_max_swapfile_size();
+
+	if (boot_cpu_has(X86_BUG_L1TF)) {
+		/* Limit the swap file size to MAX_PA/2 for L1TF workaround */
+		pages = min_t(unsigned long, maxpa_pfn_bit(1), pages);
+	}
+	return pages;
+}
diff --git a/include/linux/swapfile.h b/include/linux/swapfile.h
index 06bd7b096167..e06febf62978 100644
--- a/include/linux/swapfile.h
+++ b/include/linux/swapfile.h
@@ -10,5 +10,7 @@ extern spinlock_t swap_lock;
 extern struct plist_head swap_active_head;
 extern struct swap_info_struct *swap_info[];
 extern int try_to_unuse(unsigned int, bool, unsigned long);
+extern unsigned long generic_max_swapfile_size(void);
+extern unsigned long max_swapfile_size(void);
 
 #endif /* _LINUX_SWAPFILE_H */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index cc2cf04d9018..413f48424194 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2909,6 +2909,33 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode)
 	return 0;
 }
 
+
+/*
+ * Find out how many pages are allowed for a single swap
+ * device. There are two limiting factors: 1) the number
+ * of bits for the swap offset in the swp_entry_t type, and
+ * 2) the number of bits in the swap pte as defined by the
+ * different architectures. In order to find the
+ * largest possible bit mask, a swap entry with swap type 0
+ * and swap offset ~0UL is created, encoded to a swap pte,
+ * decoded to a swp_entry_t again, and finally the swap
+ * offset is extracted. This will mask all the bits from
+ * the initial ~0UL mask that can't be encoded in either
+ * the swp_entry_t or the architecture definition of a
+ * swap pte.
+ */
+unsigned long generic_max_swapfile_size(void)
+{
+	return swp_offset(pte_to_swp_entry(
+			swp_entry_to_pte(swp_entry(0, ~0UL)))) + 1;
+}
+
+/* Can be overridden by an architecture for additional checks. */
+__weak unsigned long max_swapfile_size(void)
+{
+	return generic_max_swapfile_size();
+}
+
 static unsigned long read_swap_header(struct swap_info_struct *p,
 					union swap_header *swap_header,
 					struct inode *inode)
@@ -2944,22 +2971,7 @@ static unsigned long read_swap_header(struct swap_info_struct *p,
 	p->cluster_next = 1;
 	p->cluster_nr = 0;
 
-	/*
-	 * Find out how many pages are allowed for a single swap
-	 * device. There are two limiting factors: 1) the number
-	 * of bits for the swap offset in the swp_entry_t type, and
-	 * 2) the number of bits in the swap pte as defined by the
-	 * different architectures. In order to find the
-	 * largest possible bit mask, a swap entry with swap type 0
-	 * and swap offset ~0UL is created, encoded to a swap pte,
-	 * decoded to a swp_entry_t again, and finally the swap
-	 * offset is extracted. This will mask all the bits from
-	 * the initial ~0UL mask that can't be encoded in either
-	 * the swp_entry_t or the architecture definition of a
-	 * swap pte.
-	 */
-	maxpages = swp_offset(pte_to_swp_entry(
-			swp_entry_to_pte(swp_entry(0, ~0UL)))) + 1;
+	maxpages = max_swapfile_size();
 	last_page = swap_header->info.last_page;
 	if (!last_page) {
 		pr_warn("Empty swap-file\n");
-- 
2.14.3

Re: [PATCH v4 7/8] L1TFv4 2

[Thomas Gleixner]

On Wed, 9 May 2018, speck for ak_at_linux.intel.com wrote:
  
> +static inline u64 maxpa_pfn_bit(int offset)

What is offset for? All callsites I found so far are handing in '1'.

Aside of that the function and the argument name are not really
intuitive. It suggests that it returns a bit number (at least that's how I
read it in the first place) with an address offset from the maximal
physical address while it returns the PFN of

    maximal physical address >> offset

> +{
> +	return BIT_ULL(boot_cpu_data.x86_phys_bits - offset - PAGE_SHIFT);
> +}

> +unsigned long max_swapfile_size(void)
> +{
> +	unsigned long pages;
> +
> +	pages = generic_max_swapfile_size();
> +
> +	if (boot_cpu_has(X86_BUG_L1TF)) {
> +		/* Limit the swap file size to MAX_PA/2 for L1TF workaround */
> +		pages = min_t(unsigned long, maxpa_pfn_bit(1), pages);

So here you use it for comparison as the maximum number of pages which fit
into MAX_PA/2. But in the next patch you use it to check whether the
returned bit mask is set in a pfn. 

> +/*
> +	if ((pfn & maxpa_pfn_bit(1)) && !capable(CAP_SYS_ADMIN))
> +		return false;

Both use cases are correct, but I had to look more than twice.

Aside of that returning u64 from that function is wrong. It works on 64 bit
but on 32bit 

    pages = min_t(unsigned long, maxpa_pfn_bit(1), pages);

will barf because unsigned long != u64.

I'd suggest to make it something like this:

static inline unsigned long l1tf_pfn_limit(void)
{
	return BIT_UL(boot_cpu_data.x86_phys_bits - offset - PAGE_SHIFT)  - 1;
}

and at the call sites then do:

	pages = min_t(unsigned long, l1tf_pfn_limit()  + 1, pages);

and

	if (pfn > l1tf_pnf_limit() && !capable(CAP_SYS_ADMIN))

which makes it obvious what that is about. 

Hmm?

Thanks,

	tglx

Re: [PATCH v4 7/8] L1TFv4 2

[Thomas Gleixner]

On Wed, 9 May 2018, speck for ak_at_linux.intel.com wrote:
>  
> +
> +/*
> + * Find out how many pages are allowed for a single swap
> + * device. There are two limiting factors: 1) the number
> + * of bits for the swap offset in the swp_entry_t type, and
> + * 2) the number of bits in the swap pte as defined by the
> + * different architectures. In order to find the
> + * largest possible bit mask, a swap entry with swap type 0
> + * and swap offset ~0UL is created, encoded to a swap pte,
> + * decoded to a swp_entry_t again, and finally the swap
> + * offset is extracted. This will mask all the bits from
> + * the initial ~0UL mask that can't be encoded in either
> + * the swp_entry_t or the architecture definition of a
> + * swap pte.

Can you please reformat that comment so it used the full width instead of
just moving and unindenting it?

Thanks,

	tglx

[MODERATED] [PATCH v4 8/8] L1TFv4 5

[ak]

For L1TF PROT_NONE mappings are protected by inverting the PFN in the
page table entry. This sets the high bits in the CPU's address space,
thus making sure to point to not point an unmapped entry to valid
cached memory.

Some server system BIOS put the MMIO mappings high up in the physical
address space. If such an high mapping was mapped to an unprivileged
user they could attack low memory by setting such a mapping to
PROT_NONE. This could happen through a special device driver
which is not access protected. Normal /dev/mem is of course
access protect.

To avoid this we forbid PROT_NONE mappings or mprotect for high MMIO
mappings.

Valid page mappings are allowed because the system is then unsafe
anyways.

We don't expect users to commonly use PROT_NONE on MMIO. But
to minimize any impact here we only do this if the mapping actually
refers to a high MMIO address (defined as the MAX_PA-1 bit being set),
and also skip the check for root.

For mmaps this is straight forward and can be handled in vm_insert_pfn
and in remap_pfn_range().

For mprotect it's a bit trickier. At the point we're looking at the
actual PTEs a lot of state has been changed and would be difficult
to undo on an error. Since this is a uncommon case we use a separate
early page talk walk pass for MMIO PROT_NONE mappings that
checks for this condition early. For non MMIO and non PROT_NONE
there are no changes.

v2: Use new helpers added earlier
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/pgtable.h |  4 ++++
 arch/x86/mm/mmap.c             | 19 ++++++++++++++++
 include/asm-generic/pgtable.h  | 12 +++++++++++
 mm/memory.c                    | 37 ++++++++++++++++++++++---------
 mm/mprotect.c                  | 49 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 111 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index f811e3257e87..338897c3b36f 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1333,6 +1333,10 @@ static inline bool pud_access_permitted(pud_t pud, bool write)
 	return __pte_access_permitted(pud_val(pud), write);
 }
 
+#define __HAVE_ARCH_PFN_MODIFY_ALLOWED 1
+extern bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot);
+static inline bool arch_has_pfn_modify_check(void) { return true; }
+
 #include <asm-generic/pgtable.h>
 #endif	/* __ASSEMBLY__ */
 
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 48c591251600..369b67226f81 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -240,3 +240,22 @@ int valid_mmap_phys_addr_range(unsigned long pfn, size_t count)
 
 	return phys_addr_valid(addr + count - 1);
 }
+
+/*
+ * Only allow root to set high MMIO mappings to PROT_NONE.
+ * This prevents an unpriv. user to set them to PROT_NONE and invert
+ * them, then pointing to valid memory for L1TF speculation.
+ */
+bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
+{
+	if (!boot_cpu_has(X86_BUG_L1TF))
+		return true;
+	if (__pte_needs_invert(pgprot_val(prot)))
+		return true;
+	/* If it's real memory always allow */
+	if (pfn_valid(pfn))
+		return true;
+	if ((pfn & maxpa_pfn_bit(1)) && !capable(CAP_SYS_ADMIN))
+		return false;
+	return true;
+}
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index f59639afaa39..0ecc1197084b 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -1097,4 +1097,16 @@ static inline void init_espfix_bsp(void) { }
 #endif
 #endif
 
+#ifndef __HAVE_ARCH_PFN_MODIFY_ALLOWED
+static inline bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
+{
+	return true;
+}
+
+static inline bool arch_has_pfn_modify_check(void)
+{
+	return false;
+}
+#endif
+
 #endif /* _ASM_GENERIC_PGTABLE_H */
diff --git a/mm/memory.c b/mm/memory.c
index 01f5464e0fd2..fe497cecd2ab 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1891,6 +1891,9 @@ int vm_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,
 	if (addr < vma->vm_start || addr >= vma->vm_end)
 		return -EFAULT;
 
+	if (!pfn_modify_allowed(pfn, pgprot))
+		return -EACCES;
+
 	track_pfn_insert(vma, &pgprot, __pfn_to_pfn_t(pfn, PFN_DEV));
 
 	ret = insert_pfn(vma, addr, __pfn_to_pfn_t(pfn, PFN_DEV), pgprot,
@@ -1926,6 +1929,9 @@ static int __vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
 
 	track_pfn_insert(vma, &pgprot, pfn);
 
+	if (!pfn_modify_allowed(pfn_t_to_pfn(pfn), pgprot))
+		return -EACCES;
+
 	/*
 	 * If we don't have pte special, then we have to use the pfn_valid()
 	 * based VM_MIXEDMAP scheme (see vm_normal_page), and thus we *must*
@@ -1973,6 +1979,7 @@ static int remap_pte_range(struct mm_struct *mm, pmd_t *pmd,
 {
 	pte_t *pte;
 	spinlock_t *ptl;
+	int err = 0;
 
 	pte = pte_alloc_map_lock(mm, pmd, addr, &ptl);
 	if (!pte)
@@ -1980,12 +1987,16 @@ static int remap_pte_range(struct mm_struct *mm, pmd_t *pmd,
 	arch_enter_lazy_mmu_mode();
 	do {
 		BUG_ON(!pte_none(*pte));
+		if (!pfn_modify_allowed(pfn, prot)) {
+			err = -EACCES;
+			break;
+		}
 		set_pte_at(mm, addr, pte, pte_mkspecial(pfn_pte(pfn, prot)));
 		pfn++;
 	} while (pte++, addr += PAGE_SIZE, addr != end);
 	arch_leave_lazy_mmu_mode();
 	pte_unmap_unlock(pte - 1, ptl);
-	return 0;
+	return err;
 }
 
 static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
@@ -1994,6 +2005,7 @@ static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
 {
 	pmd_t *pmd;
 	unsigned long next;
+	int err;
 
 	pfn -= addr >> PAGE_SHIFT;
 	pmd = pmd_alloc(mm, pud, addr);
@@ -2002,9 +2014,10 @@ static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
 	VM_BUG_ON(pmd_trans_huge(*pmd));
 	do {
 		next = pmd_addr_end(addr, end);
-		if (remap_pte_range(mm, pmd, addr, next,
-				pfn + (addr >> PAGE_SHIFT), prot))
-			return -ENOMEM;
+		err = remap_pte_range(mm, pmd, addr, next,
+				pfn + (addr >> PAGE_SHIFT), prot);
+		if (err)
+			return err;
 	} while (pmd++, addr = next, addr != end);
 	return 0;
 }
@@ -2015,6 +2028,7 @@ static inline int remap_pud_range(struct mm_struct *mm, p4d_t *p4d,
 {
 	pud_t *pud;
 	unsigned long next;
+	int err;
 
 	pfn -= addr >> PAGE_SHIFT;
 	pud = pud_alloc(mm, p4d, addr);
@@ -2022,9 +2036,10 @@ static inline int remap_pud_range(struct mm_struct *mm, p4d_t *p4d,
 		return -ENOMEM;
 	do {
 		next = pud_addr_end(addr, end);
-		if (remap_pmd_range(mm, pud, addr, next,
-				pfn + (addr >> PAGE_SHIFT), prot))
-			return -ENOMEM;
+		err = remap_pmd_range(mm, pud, addr, next,
+				pfn + (addr >> PAGE_SHIFT), prot);
+		if (err)
+			return err;
 	} while (pud++, addr = next, addr != end);
 	return 0;
 }
@@ -2035,6 +2050,7 @@ static inline int remap_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 {
 	p4d_t *p4d;
 	unsigned long next;
+	int err;
 
 	pfn -= addr >> PAGE_SHIFT;
 	p4d = p4d_alloc(mm, pgd, addr);
@@ -2042,9 +2058,10 @@ static inline int remap_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 		return -ENOMEM;
 	do {
 		next = p4d_addr_end(addr, end);
-		if (remap_pud_range(mm, p4d, addr, next,
-				pfn + (addr >> PAGE_SHIFT), prot))
-			return -ENOMEM;
+		err = remap_pud_range(mm, p4d, addr, next,
+				pfn + (addr >> PAGE_SHIFT), prot);
+		if (err)
+			return err;
 	} while (p4d++, addr = next, addr != end);
 	return 0;
 }
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 625608bc8962..6d331620b9e5 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -306,6 +306,42 @@ unsigned long change_protection(struct vm_area_struct *vma, unsigned long start,
 	return pages;
 }
 
+static int prot_none_pte_entry(pte_t *pte, unsigned long addr,
+			       unsigned long next, struct mm_walk *walk)
+{
+	return pfn_modify_allowed(pte_pfn(*pte), *(pgprot_t *)(walk->private)) ?
+		0 : -EACCES;
+}
+
+static int prot_none_hugetlb_entry(pte_t *pte, unsigned long hmask,
+				   unsigned long addr, unsigned long next,
+				   struct mm_walk *walk)
+{
+	return pfn_modify_allowed(pte_pfn(*pte), *(pgprot_t *)(walk->private)) ?
+		0 : -EACCES;
+}
+
+static int prot_none_test(unsigned long addr, unsigned long next,
+			  struct mm_walk *walk)
+{
+	return 0;
+}
+
+static int prot_none_walk(struct vm_area_struct *vma, unsigned long start,
+			   unsigned long end, unsigned long newflags)
+{
+	pgprot_t new_pgprot = vm_get_page_prot(newflags);
+	struct mm_walk prot_none_walk = {
+		.pte_entry = prot_none_pte_entry,
+		.hugetlb_entry = prot_none_hugetlb_entry,
+		.test_walk = prot_none_test,
+		.mm = current->mm,
+		.private = &new_pgprot,
+	};
+
+	return walk_page_range(start, end, &prot_none_walk);
+}
+
 int
 mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	unsigned long start, unsigned long end, unsigned long newflags)
@@ -323,6 +359,19 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 		return 0;
 	}
 
+	/*
+	 * Do PROT_NONE PFN permission checks here when we can still
+	 * bail out without undoing a lot of state. This is a rather
+	 * uncommon case, so doesn't need to be very optimized.
+	 */
+	if (arch_has_pfn_modify_check() &&
+	    (vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) &&
+	    (newflags & (VM_READ|VM_WRITE|VM_EXEC)) == 0) {
+		error = prot_none_walk(vma, start, end, newflags);
+		if (error)
+			return error;
+	}
+
 	/*
 	 * If we make a private mapping writable we increase our commit;
 	 * but (without finer accounting) cannot reduce our commit if we
-- 
2.14.3

[MODERATED] Re: [PATCH v4 8/8] L1TFv4 5

[Andi Kleen]

Managed to add a bug in the last cleanup of this patch, inverting
a condition. Here's an updated patch.

---

mm, l1tf: Disallow non privileged high MMIO PROT_NONE mappings

For L1TF PROT_NONE mappings are protected by inverting the PFN in the
page table entry. This sets the high bits in the CPU's address space,
thus making sure to point to not point an unmapped entry to valid
cached memory.

Some server system BIOS put the MMIO mappings high up in the physical
address space. If such an high mapping was mapped to an unprivileged
user they could attack low memory by setting such a mapping to
PROT_NONE. This could happen through a special device driver
which is not access protected. Normal /dev/mem is of course
access protect.

To avoid this we forbid PROT_NONE mappings or mprotect for high MMIO
mappings.

Valid page mappings are allowed because the system is then unsafe
anyways.

We don't expect users to commonly use PROT_NONE on MMIO. But
to minimize any impact here we only do this if the mapping actually
refers to a high MMIO address (defined as the MAX_PA-1 bit being set),
and also skip the check for root.

For mmaps this is straight forward and can be handled in vm_insert_pfn
and in remap_pfn_range().

For mprotect it's a bit trickier. At the point we're looking at the
actual PTEs a lot of state has been changed and would be difficult
to undo on an error. Since this is a uncommon case we use a separate
early page talk walk pass for MMIO PROT_NONE mappings that
checks for this condition early. For non MMIO and non PROT_NONE
there are no changes.

v2: Use new helpers added earlier
v3: Fix inverted check added in v3
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/pgtable.h |  4 ++++
 arch/x86/mm/mmap.c             | 19 ++++++++++++++++
 include/asm-generic/pgtable.h  | 12 +++++++++++
 mm/memory.c                    | 37 ++++++++++++++++++++++---------
 mm/mprotect.c                  | 49 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 111 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index f811e3257e87..338897c3b36f 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1333,6 +1333,10 @@ static inline bool pud_access_permitted(pud_t pud, bool write)
 	return __pte_access_permitted(pud_val(pud), write);
 }
 
+#define __HAVE_ARCH_PFN_MODIFY_ALLOWED 1
+extern bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot);
+static inline bool arch_has_pfn_modify_check(void) { return true; }
+
 #include <asm-generic/pgtable.h>
 #endif	/* __ASSEMBLY__ */
 
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 48c591251600..3f2e59b928f4 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -240,3 +240,22 @@ int valid_mmap_phys_addr_range(unsigned long pfn, size_t count)
 
 	return phys_addr_valid(addr + count - 1);
 }
+
+/*
+ * Only allow root to set high MMIO mappings to PROT_NONE.
+ * This prevents an unpriv. user to set them to PROT_NONE and invert
+ * them, then pointing to valid memory for L1TF speculation.
+ */
+bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
+{
+	if (!boot_cpu_has(X86_BUG_L1TF))
+		return true;
+	if (!__pte_needs_invert(pgprot_val(prot)))
+		return true;
+	/* If it's real memory always allow */
+	if (pfn_valid(pfn))
+		return true;
+	if ((pfn & maxpa_pfn_bit(1)) && !capable(CAP_SYS_ADMIN))
+		return false;
+	return true;
+}
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index f59639afaa39..0ecc1197084b 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -1097,4 +1097,16 @@ static inline void init_espfix_bsp(void) { }
 #endif
 #endif
 
+#ifndef __HAVE_ARCH_PFN_MODIFY_ALLOWED
+static inline bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
+{
+	return true;
+}
+
+static inline bool arch_has_pfn_modify_check(void)
+{
+	return false;
+}
+#endif
+
 #endif /* _ASM_GENERIC_PGTABLE_H */
diff --git a/mm/memory.c b/mm/memory.c
index 01f5464e0fd2..fe497cecd2ab 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1891,6 +1891,9 @@ int vm_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,
 	if (addr < vma->vm_start || addr >= vma->vm_end)
 		return -EFAULT;
 
+	if (!pfn_modify_allowed(pfn, pgprot))
+		return -EACCES;
+
 	track_pfn_insert(vma, &pgprot, __pfn_to_pfn_t(pfn, PFN_DEV));
 
 	ret = insert_pfn(vma, addr, __pfn_to_pfn_t(pfn, PFN_DEV), pgprot,
@@ -1926,6 +1929,9 @@ static int __vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
 
 	track_pfn_insert(vma, &pgprot, pfn);
 
+	if (!pfn_modify_allowed(pfn_t_to_pfn(pfn), pgprot))
+		return -EACCES;
+
 	/*
 	 * If we don't have pte special, then we have to use the pfn_valid()
 	 * based VM_MIXEDMAP scheme (see vm_normal_page), and thus we *must*
@@ -1973,6 +1979,7 @@ static int remap_pte_range(struct mm_struct *mm, pmd_t *pmd,
 {
 	pte_t *pte;
 	spinlock_t *ptl;
+	int err = 0;
 
 	pte = pte_alloc_map_lock(mm, pmd, addr, &ptl);
 	if (!pte)
@@ -1980,12 +1987,16 @@ static int remap_pte_range(struct mm_struct *mm, pmd_t *pmd,
 	arch_enter_lazy_mmu_mode();
 	do {
 		BUG_ON(!pte_none(*pte));
+		if (!pfn_modify_allowed(pfn, prot)) {
+			err = -EACCES;
+			break;
+		}
 		set_pte_at(mm, addr, pte, pte_mkspecial(pfn_pte(pfn, prot)));
 		pfn++;
 	} while (pte++, addr += PAGE_SIZE, addr != end);
 	arch_leave_lazy_mmu_mode();
 	pte_unmap_unlock(pte - 1, ptl);
-	return 0;
+	return err;
 }
 
 static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
@@ -1994,6 +2005,7 @@ static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
 {
 	pmd_t *pmd;
 	unsigned long next;
+	int err;
 
 	pfn -= addr >> PAGE_SHIFT;
 	pmd = pmd_alloc(mm, pud, addr);
@@ -2002,9 +2014,10 @@ static inline int remap_pmd_range(struct mm_struct *mm, pud_t *pud,
 	VM_BUG_ON(pmd_trans_huge(*pmd));
 	do {
 		next = pmd_addr_end(addr, end);
-		if (remap_pte_range(mm, pmd, addr, next,
-				pfn + (addr >> PAGE_SHIFT), prot))
-			return -ENOMEM;
+		err = remap_pte_range(mm, pmd, addr, next,
+				pfn + (addr >> PAGE_SHIFT), prot);
+		if (err)
+			return err;
 	} while (pmd++, addr = next, addr != end);
 	return 0;
 }
@@ -2015,6 +2028,7 @@ static inline int remap_pud_range(struct mm_struct *mm, p4d_t *p4d,
 {
 	pud_t *pud;
 	unsigned long next;
+	int err;
 
 	pfn -= addr >> PAGE_SHIFT;
 	pud = pud_alloc(mm, p4d, addr);
@@ -2022,9 +2036,10 @@ static inline int remap_pud_range(struct mm_struct *mm, p4d_t *p4d,
 		return -ENOMEM;
 	do {
 		next = pud_addr_end(addr, end);
-		if (remap_pmd_range(mm, pud, addr, next,
-				pfn + (addr >> PAGE_SHIFT), prot))
-			return -ENOMEM;
+		err = remap_pmd_range(mm, pud, addr, next,
+				pfn + (addr >> PAGE_SHIFT), prot);
+		if (err)
+			return err;
 	} while (pud++, addr = next, addr != end);
 	return 0;
 }
@@ -2035,6 +2050,7 @@ static inline int remap_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 {
 	p4d_t *p4d;
 	unsigned long next;
+	int err;
 
 	pfn -= addr >> PAGE_SHIFT;
 	p4d = p4d_alloc(mm, pgd, addr);
@@ -2042,9 +2058,10 @@ static inline int remap_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 		return -ENOMEM;
 	do {
 		next = p4d_addr_end(addr, end);
-		if (remap_pud_range(mm, p4d, addr, next,
-				pfn + (addr >> PAGE_SHIFT), prot))
-			return -ENOMEM;
+		err = remap_pud_range(mm, p4d, addr, next,
+				pfn + (addr >> PAGE_SHIFT), prot);
+		if (err)
+			return err;
 	} while (p4d++, addr = next, addr != end);
 	return 0;
 }
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 625608bc8962..6d331620b9e5 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -306,6 +306,42 @@ unsigned long change_protection(struct vm_area_struct *vma, unsigned long start,
 	return pages;
 }
 
+static int prot_none_pte_entry(pte_t *pte, unsigned long addr,
+			       unsigned long next, struct mm_walk *walk)
+{
+	return pfn_modify_allowed(pte_pfn(*pte), *(pgprot_t *)(walk->private)) ?
+		0 : -EACCES;
+}
+
+static int prot_none_hugetlb_entry(pte_t *pte, unsigned long hmask,
+				   unsigned long addr, unsigned long next,
+				   struct mm_walk *walk)
+{
+	return pfn_modify_allowed(pte_pfn(*pte), *(pgprot_t *)(walk->private)) ?
+		0 : -EACCES;
+}
+
+static int prot_none_test(unsigned long addr, unsigned long next,
+			  struct mm_walk *walk)
+{
+	return 0;
+}
+
+static int prot_none_walk(struct vm_area_struct *vma, unsigned long start,
+			   unsigned long end, unsigned long newflags)
+{
+	pgprot_t new_pgprot = vm_get_page_prot(newflags);
+	struct mm_walk prot_none_walk = {
+		.pte_entry = prot_none_pte_entry,
+		.hugetlb_entry = prot_none_hugetlb_entry,
+		.test_walk = prot_none_test,
+		.mm = current->mm,
+		.private = &new_pgprot,
+	};
+
+	return walk_page_range(start, end, &prot_none_walk);
+}
+
 int
 mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	unsigned long start, unsigned long end, unsigned long newflags)
@@ -323,6 +359,19 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 		return 0;
 	}
 
+	/*
+	 * Do PROT_NONE PFN permission checks here when we can still
+	 * bail out without undoing a lot of state. This is a rather
+	 * uncommon case, so doesn't need to be very optimized.
+	 */
+	if (arch_has_pfn_modify_check() &&
+	    (vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) &&
+	    (newflags & (VM_READ|VM_WRITE|VM_EXEC)) == 0) {
+		error = prot_none_walk(vma, start, end, newflags);
+		if (error)
+			return error;
+	}
+
 	/*
 	 * If we make a private mapping writable we increase our commit;
 	 * but (without finer accounting) cannot reduce our commit if we
-- 
2.14.3

[MODERATED] Re: [PATCH v4 0/8] L1TFv4 0

[Andi Kleen]

[-- Attachment #1: Type: text/plain, Size: 48 bytes --]


And here's a mbox for easier review/applying



[-- Attachment #2: m --]
[-- Type: application/vnd.wolfram.mathematica.package, Size: 41131 bytes --]

[MODERATED] Re: ***UNCHECKED*** [PATCH v4 0/8] L1TFv4 0

[Vlastimil Babka]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On 05/09/2018 11:25 PM, speck for ak_at_linux.intel.com wrote:
> From: Andi Kleen <ak@linux.intel.com>
> Subject:  *** SUBJECT HERE ***
> 
> Here's the updated L1TF native OS patchkit to mitigate speculation
> into unmapped PTEs.  This is stabilizing now.
> 
> This version has over v3:
> - Cleanups/changes from code review

I don't see my changelog fixups and acks for patch 2/8 and 3/8. Not that
they would be important, but perhaps that means you sent a wrong version
or something...

[MODERATED] Re: ***UNCHECKED*** [PATCH v4 0/8] L1TFv4 0

[Michal Hocko]

On Thu 10-05-18 08:47:20, speck for Vlastimil Babka wrote:
> On 05/09/2018 11:25 PM, speck for ak_at_linux.intel.com wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > Subject:  *** SUBJECT HERE ***
> > 
> > Here's the updated L1TF native OS patchkit to mitigate speculation
> > into unmapped PTEs.  This is stabilizing now.
> > 
> > This version has over v3:
> > - Cleanups/changes from code review
> 
> I don't see my changelog fixups and acks for patch 2/8 and 3/8. Not that
> they would be important, but perhaps that means you sent a wrong version
> or something...

Patches 1 and 7 are missing my acks as well.

-- 
Michal Hocko
SUSE Labs

[MODERATED] Re: ***UNCHECKED*** [PATCH v4 0/8] L1TFv4 0

[Andi Kleen]

On Thu, May 10, 2018 at 10:04:17AM +0200, speck for Michal Hocko wrote:
> On Thu 10-05-18 08:47:20, speck for Vlastimil Babka wrote:
> > On 05/09/2018 11:25 PM, speck for ak_at_linux.intel.com wrote:
> > > From: Andi Kleen <ak@linux.intel.com>
> > > Subject:  *** SUBJECT HERE ***
> > > 
> > > Here's the updated L1TF native OS patchkit to mitigate speculation
> > > into unmapped PTEs.  This is stabilizing now.
> > > 
> > > This version has over v3:
> > > - Cleanups/changes from code review
> > 
> > I don't see my changelog fixups and acks for patch 2/8 and 3/8. Not that
> > they would be important, but perhaps that means you sent a wrong version
> > or something...
> 
> Patches 1 and 7 are missing my acks as well.

Yes I was missing some acks. Will resend.

The code should be the right version.

-Andi

[MODERATED] Re: [PATCH v4 0/8] L1TFv4 0

[Borislav Petkov]

On Thu, May 10, 2018 at 09:07:22AM -0700, speck for Andi Kleen wrote:
> The code should be the right version.

Well, it still needs to be rebased on current speck/master.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [PATCH v4 0/8] L1TFv4 0

[Andi Kleen]

On Thu, May 10, 2018 at 06:57:59PM +0200, speck for Borislav Petkov wrote:
> On Thu, May 10, 2018 at 09:07:22AM -0700, speck for Andi Kleen wrote:
> > The code should be the right version.
> 
> Well, it still needs to be rebased on current speck/master.

Keep in mind that this one will be embargoed much longer than SSB,
so likely it will exist on its own for a long time. 

I think it's only the conflicts in the feature bits anyways.

The rebase should be fairly straight forward, so it seemed logistically
better to have independendent patchkits for now.

Let me know if anyone disagrees.

-Andi

Re: [PATCH v4 0/8] L1TFv4 0

[Thomas Gleixner]

On Thu, 10 May 2018, speck for Andi Kleen wrote:
> On Thu, May 10, 2018 at 06:57:59PM +0200, speck for Borislav Petkov wrote:
> > On Thu, May 10, 2018 at 09:07:22AM -0700, speck for Andi Kleen wrote:
> > > The code should be the right version.
> > 
> > Well, it still needs to be rebased on current speck/master.
> 
> Keep in mind that this one will be embargoed much longer than SSB,
> so likely it will exist on its own for a long time. 
> 
> I think it's only the conflicts in the feature bits anyways.
> 
> The rebase should be fairly straight forward, so it seemed logistically
> better to have independendent patchkits for now.

Well even if it will be embargoed for a long time, it still will be on top
of the SSB stuff and logistically it's way simpler if you rebase the patch
series on top of that instead of having anybody else doing it over and
over.

Thanks,

	tglx

[MODERATED] Re: [PATCH v4 0/8] L1TFv4 0

[Andi Kleen]

> Well even if it will be embargoed for a long time, it still will be on top
> of the SSB stuff and logistically it's way simpler if you rebase the patch
> series on top of that instead of having anybody else doing it over and
> over.

Ok. Will do.

-Andi