[SRU][Bionic][PATCH 0/3] Fixes for LP:1788563

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 0/3] Fixes for LP:1788563

Joseph Salisbury-3
BugLink: https://bugs.launchpad.net/bugs/1789834

== SRU Justification ==
This regression has been reported in multiple bugs and affects Trust,
Xenial and Bionic.  All releases need different backports, so T and X
will be sent in separate SRU requests.

Due to this regression L1TF mitigation not effective.  

== Fixes ==
9df9516940a6 ("x86/speculation/l1tf: Fix overflow in l1tf_pfn_limit() on 32bit")
b0a182f87568 ("x86/speculation/l1tf: Fix off-by-one error when warning that system has too much RAM")
cc51e5428ea5 ("x86/speculation/l1tf: Increase l1tf memory limit for Nehalem+")

== Regression Potential ==
Low.  These are security fixes and have all been cc'd to upstream
stable, so they have had additional upstream review.

== Test Case ==
A test kernel was built with these patches and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Andi Kleen (1):
  x86/speculation/l1tf: Increase l1tf memory limit for Nehalem+

Vlastimil Babka (2):
  x86/speculation/l1tf: Fix overflow in l1tf_pfn_limit() on 32bit
  x86/speculation/l1tf: Fix off-by-one error when warning that system
    has too much RAM

 arch/x86/include/asm/processor.h |  6 ++++--
 arch/x86/kernel/cpu/bugs.c       | 46 +++++++++++++++++++++++++++++++++++-----
 arch/x86/mm/init.c               |  4 ++--
 arch/x86/mm/mmap.c               |  2 +-
 4 files changed, 48 insertions(+), 10 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 1/3] x86/speculation/l1tf: Fix overflow in l1tf_pfn_limit() on 32bit

Joseph Salisbury-3
From: Vlastimil Babka <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1789834

On 32bit PAE kernels on 64bit hardware with enough physical bits,
l1tf_pfn_limit() will overflow unsigned long. This in turn affects
max_swapfile_size() and can lead to swapon returning -EINVAL. This has been
observed in a 32bit guest with 42 bits physical address size, where
max_swapfile_size() overflows exactly to 1 << 32, thus zero, and produces
the following warning to dmesg:

[    6.396845] Truncating oversized swap area, only using 0k out of 2047996k

Fix this by using unsigned long long instead.

Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
Fixes: 377eeaa8e11f ("x86/speculation/l1tf: Limit swap file size to MAX_PA/2")
Reported-by: Dominique Leuenberger <[hidden email]>
Reported-by: Adrian Schroeter <[hidden email]>
Signed-off-by: Vlastimil Babka <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: Andi Kleen <[hidden email]>
Acked-by: Michal Hocko <[hidden email]>
Cc: "H . Peter Anvin" <[hidden email]>
Cc: Linus Torvalds <[hidden email]>
Cc: Dave Hansen <[hidden email]>
Cc: Michal Hocko <[hidden email]>
Cc: [hidden email]
Link: https://lkml.kernel.org/r/20180820095835.5298-1-vbabka@...

(cherry picked from commit 9df9516940a61d29aedf4d91b483ca6597e7d480)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 arch/x86/include/asm/processor.h | 4 ++--
 arch/x86/mm/init.c               | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 23e6a13..57c51ec 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -181,9 +181,9 @@ extern const struct seq_operations cpuinfo_op;
 
 extern void cpu_detect(struct cpuinfo_x86 *c);
 
-static inline unsigned long l1tf_pfn_limit(void)
+static inline unsigned long long l1tf_pfn_limit(void)
 {
- return BIT(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
+ return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
 }
 
 extern void early_cpu_init(void);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 78129a1..6b419ac 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -895,7 +895,7 @@ unsigned long max_swapfile_size(void)
 
  if (boot_cpu_has_bug(X86_BUG_L1TF)) {
  /* Limit the swap file size to MAX_PA/2 for L1TF workaround */
- unsigned long l1tf_limit = l1tf_pfn_limit() + 1;
+ unsigned long long l1tf_limit = l1tf_pfn_limit() + 1;
  /*
  * We encode swap offsets also with 3 bits below those for pfn
  * which makes the usable limit higher.
@@ -903,7 +903,7 @@ unsigned long max_swapfile_size(void)
 #if CONFIG_PGTABLE_LEVELS > 2
  l1tf_limit <<= PAGE_SHIFT - SWP_OFFSET_FIRST_BIT;
 #endif
- pages = min_t(unsigned long, l1tf_limit, pages);
+ pages = min_t(unsigned long long, l1tf_limit, pages);
  }
  return pages;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 2/3] x86/speculation/l1tf: Fix off-by-one error when warning that system has too much RAM

Joseph Salisbury-3
In reply to this post by Joseph Salisbury-3
From: Vlastimil Babka <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1789834

Two users have reported [1] that they have an "extremely unlikely" system
with more than MAX_PA/2 memory and L1TF mitigation is not effective. In
fact it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to
holes in the e820 map, the main region is almost 500MB over the 32GB limit:

[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000081effffff] usable

Suggestions to use 'mem=32G' to enable the L1TF mitigation while losing the
500MB revealed, that there's an off-by-one error in the check in
l1tf_select_mitigation().

l1tf_pfn_limit() returns the last usable pfn (inclusive) and the range
check in the mitigation path does not take this into account.

Instead of amending the range check, make l1tf_pfn_limit() return the first
PFN which is over the limit which is less error prone. Adjust the other
users accordingly.

[1] https://bugzilla.suse.com/show_bug.cgi?id=1105536

Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
Reported-by: George Anchev <[hidden email]>
Reported-by: Christopher Snowhill <[hidden email]>
Signed-off-by: Vlastimil Babka <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Cc: "H . Peter Anvin" <[hidden email]>
Cc: Linus Torvalds <[hidden email]>
Cc: Andi Kleen <[hidden email]>
Cc: Dave Hansen <[hidden email]>
Cc: Michal Hocko <[hidden email]>
Cc: [hidden email]
Link: https://lkml.kernel.org/r/20180823134418.17008-1-vbabka@...

(cherry picked from commit b0a182f875689647b014bc01d36b340217792852)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 arch/x86/include/asm/processor.h | 2 +-
 arch/x86/mm/init.c               | 2 +-
 arch/x86/mm/mmap.c               | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 57c51ec..8c770dd 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -183,7 +183,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
 
 static inline unsigned long long l1tf_pfn_limit(void)
 {
- return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
+ return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
 }
 
 extern void early_cpu_init(void);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 6b419ac..eef30c5 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -895,7 +895,7 @@ unsigned long max_swapfile_size(void)
 
  if (boot_cpu_has_bug(X86_BUG_L1TF)) {
  /* Limit the swap file size to MAX_PA/2 for L1TF workaround */
- unsigned long long l1tf_limit = l1tf_pfn_limit() + 1;
+ unsigned long long l1tf_limit = l1tf_pfn_limit();
  /*
  * We encode swap offsets also with 3 bits below those for pfn
  * which makes the usable limit higher.
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index bfeabff..03f8ed2 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -253,7 +253,7 @@ bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
  /* If it's real memory always allow */
  if (pfn_valid(pfn))
  return true;
- if (pfn > l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
+ if (pfn >= l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
  return false;
  return true;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 3/3] x86/speculation/l1tf: Increase l1tf memory limit for Nehalem+

Joseph Salisbury-3
In reply to this post by Joseph Salisbury-3
From: Andi Kleen <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1789834

On Nehalem and newer core CPUs the CPU cache internally uses 44 bits
physical address space. The L1TF workaround is limited by this internal
cache address width, and needs to have one bit free there for the
mitigation to work.

Older client systems report only 36bit physical address space so the range
check decides that L1TF is not mitigated for a 36bit phys/32GB system with
some memory holes.

But since these actually have the larger internal cache width this warning
is bogus because it would only really be needed if the system had more than
43bits of memory.

Add a new internal x86_cache_bits field. Normally it is the same as the
physical bits field reported by CPUID, but for Nehalem and newerforce it to
be at least 44bits.

Change the L1TF memory size warning to use the new cache_bits field to
avoid bogus warnings and remove the bogus comment about memory size.

Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
Reported-by: George Anchev <[hidden email]>
Reported-by: Christopher Snowhill <[hidden email]>
Signed-off-by: Andi Kleen <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Cc: [hidden email]
Cc: [hidden email]
Cc: Michael Hocko <[hidden email]>
Cc: [hidden email]
Cc: [hidden email]
Link: https://lkml.kernel.org/r/20180824170351.34874-1-andi@...

(backported from commit cc51e5428ea54f575d49cfcede1d4cb3a72b4ec4)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 arch/x86/include/asm/processor.h |  4 +++-
 arch/x86/kernel/cpu/bugs.c       | 46 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 8c770dd..49feba0 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -132,6 +132,8 @@ struct cpuinfo_x86 {
  /* Index into per_cpu list: */
  u16 cpu_index;
  u32 microcode;
+ /* Address space bits used by the cache internally */
+ u8 x86_cache_bits;
  unsigned initialized : 1;
 } __randomize_layout;
 
@@ -183,7 +185,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
 
 static inline unsigned long long l1tf_pfn_limit(void)
 {
- return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
+ return BIT_ULL(boot_cpu_data.x86_cache_bits - 1 - PAGE_SHIFT);
 }
 
 extern void early_cpu_init(void);
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index edfc64a..df9e418 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -653,6 +653,45 @@ enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_AUTO;
 EXPORT_SYMBOL_GPL(l1tf_vmx_mitigation);
 #endif
 
+/*
+ * These CPUs all support 44bits physical address space internally in the
+ * cache but CPUID can report a smaller number of physical address bits.
+ *
+ * The L1TF mitigation uses the top most address bit for the inversion of
+ * non present PTEs. When the installed memory reaches into the top most
+ * address bit due to memory holes, which has been observed on machines
+ * which report 36bits physical address bits and have 32G RAM installed,
+ * then the mitigation range check in l1tf_select_mitigation() triggers.
+ * This is a false positive because the mitigation is still possible due to
+ * the fact that the cache uses 44bit internally. Use the cache bits
+ * instead of the reported physical bits and adjust them on the affected
+ * machines to 44bit if the reported bits are less than 44.
+ */
+static void override_cache_bits(struct cpuinfo_x86 *c)
+{
+ if (c->x86 != 6)
+ return;
+
+ switch (c->x86_model) {
+ case INTEL_FAM6_NEHALEM:
+ case INTEL_FAM6_WESTMERE:
+ case INTEL_FAM6_SANDYBRIDGE:
+ case INTEL_FAM6_IVYBRIDGE:
+ case INTEL_FAM6_HASWELL_CORE:
+ case INTEL_FAM6_HASWELL_ULT:
+ case INTEL_FAM6_HASWELL_GT3E:
+ case INTEL_FAM6_BROADWELL_CORE:
+ case INTEL_FAM6_BROADWELL_GT3E:
+ case INTEL_FAM6_SKYLAKE_MOBILE:
+ case INTEL_FAM6_SKYLAKE_DESKTOP:
+ case INTEL_FAM6_KABYLAKE_MOBILE:
+ case INTEL_FAM6_KABYLAKE_DESKTOP:
+ if (c->x86_cache_bits < 44)
+ c->x86_cache_bits = 44;
+ break;
+ }
+}
+
 static void __init l1tf_select_mitigation(void)
 {
  u64 half_pa;
@@ -660,6 +699,8 @@ static void __init l1tf_select_mitigation(void)
  if (!boot_cpu_has_bug(X86_BUG_L1TF))
  return;
 
+ override_cache_bits(&boot_cpu_data);
+
  switch (l1tf_mitigation) {
  case L1TF_MITIGATION_OFF:
  case L1TF_MITIGATION_FLUSH_NOWARN:
@@ -679,11 +720,6 @@ static void __init l1tf_select_mitigation(void)
  return;
 #endif
 
- /*
- * This is extremely unlikely to happen because almost all
- * systems have far more MAX_PA/2 than RAM can be fit into
- * DIMM slots.
- */
  half_pa = (u64)l1tf_pfn_limit() << PAGE_SHIFT;
  if (e820__mapped_any(half_pa, ULLONG_MAX - half_pa, E820_TYPE_RAM)) {
  pr_warn("System has more than MAX_PA/2 memory. L1TF mitigation not effective.\n");
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK: [SRU][Bionic][PATCH 3/3] x86/speculation/l1tf: Increase l1tf memory limit for Nehalem+

Tyler Hicks-2
On 09/10/2018 11:02 AM, Joseph Salisbury wrote:

> From: Andi Kleen <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1789834
>
> On Nehalem and newer core CPUs the CPU cache internally uses 44 bits
> physical address space. The L1TF workaround is limited by this internal
> cache address width, and needs to have one bit free there for the
> mitigation to work.
>
> Older client systems report only 36bit physical address space so the range
> check decides that L1TF is not mitigated for a 36bit phys/32GB system with
> some memory holes.
>
> But since these actually have the larger internal cache width this warning
> is bogus because it would only really be needed if the system had more than
> 43bits of memory.
>
> Add a new internal x86_cache_bits field. Normally it is the same as the
> physical bits field reported by CPUID, but for Nehalem and newerforce it to
> be at least 44bits.
>
> Change the L1TF memory size warning to use the new cache_bits field to
> avoid bogus warnings and remove the bogus comment about memory size.
>
> Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
> Reported-by: George Anchev <[hidden email]>
> Reported-by: Christopher Snowhill <[hidden email]>
> Signed-off-by: Andi Kleen <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Cc: [hidden email]
> Cc: [hidden email]
> Cc: Michael Hocko <[hidden email]>
> Cc: [hidden email]
> Cc: [hidden email]
> Link: https://lkml.kernel.org/r/20180824170351.34874-1-andi@...
>
> (backported from commit cc51e5428ea54f575d49cfcede1d4cb3a72b4ec4)
> Signed-off-by: Joseph Salisbury <[hidden email]>
> ---
>  arch/x86/include/asm/processor.h |  4 +++-
>  arch/x86/kernel/cpu/bugs.c       | 46 +++++++++++++++++++++++++++++++++++-----
>  2 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 8c770dd..49feba0 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -132,6 +132,8 @@ struct cpuinfo_x86 {
>   /* Index into per_cpu list: */
>   u16 cpu_index;
>   u32 microcode;
> + /* Address space bits used by the cache internally */
> + u8 x86_cache_bits;
>   unsigned initialized : 1;
>  } __randomize_layout;
>  
> @@ -183,7 +185,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
>  
>  static inline unsigned long long l1tf_pfn_limit(void)
>  {
> - return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
> + return BIT_ULL(boot_cpu_data.x86_cache_bits - 1 - PAGE_SHIFT);
>  }
>  
>  extern void early_cpu_init(void);
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index edfc64a..df9e418 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -653,6 +653,45 @@ enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_AUTO;
>  EXPORT_SYMBOL_GPL(l1tf_vmx_mitigation);
>  #endif
>  
> +/*
> + * These CPUs all support 44bits physical address space internally in the
> + * cache but CPUID can report a smaller number of physical address bits.
> + *
> + * The L1TF mitigation uses the top most address bit for the inversion of
> + * non present PTEs. When the installed memory reaches into the top most
> + * address bit due to memory holes, which has been observed on machines
> + * which report 36bits physical address bits and have 32G RAM installed,
> + * then the mitigation range check in l1tf_select_mitigation() triggers.
> + * This is a false positive because the mitigation is still possible due to
> + * the fact that the cache uses 44bit internally. Use the cache bits
> + * instead of the reported physical bits and adjust them on the affected
> + * machines to 44bit if the reported bits are less than 44.
> + */
> +static void override_cache_bits(struct cpuinfo_x86 *c)
> +{
> + if (c->x86 != 6)
> + return;
> +
> + switch (c->x86_model) {
> + case INTEL_FAM6_NEHALEM:
> + case INTEL_FAM6_WESTMERE:
> + case INTEL_FAM6_SANDYBRIDGE:
> + case INTEL_FAM6_IVYBRIDGE:
> + case INTEL_FAM6_HASWELL_CORE:
> + case INTEL_FAM6_HASWELL_ULT:
> + case INTEL_FAM6_HASWELL_GT3E:
> + case INTEL_FAM6_BROADWELL_CORE:
> + case INTEL_FAM6_BROADWELL_GT3E:
> + case INTEL_FAM6_SKYLAKE_MOBILE:
> + case INTEL_FAM6_SKYLAKE_DESKTOP:
> + case INTEL_FAM6_KABYLAKE_MOBILE:
> + case INTEL_FAM6_KABYLAKE_DESKTOP:
> + if (c->x86_cache_bits < 44)
> + c->x86_cache_bits = 44;
This backport is incorrect because it drops the assignment of
c->x86_cache_bits in get_cpu_address_sizes(). Without that assignment,
c->x86_cache_bits is left uninitialized in all cases except when the CPU
family is 6 and the model matches one of the ones listed above.

Tyler

> + break;
> + }
> +}
> +
>  static void __init l1tf_select_mitigation(void)
>  {
>   u64 half_pa;
> @@ -660,6 +699,8 @@ static void __init l1tf_select_mitigation(void)
>   if (!boot_cpu_has_bug(X86_BUG_L1TF))
>   return;
>  
> + override_cache_bits(&boot_cpu_data);
> +
>   switch (l1tf_mitigation) {
>   case L1TF_MITIGATION_OFF:
>   case L1TF_MITIGATION_FLUSH_NOWARN:
> @@ -679,11 +720,6 @@ static void __init l1tf_select_mitigation(void)
>   return;
>  #endif
>  
> - /*
> - * This is extremely unlikely to happen because almost all
> - * systems have far more MAX_PA/2 than RAM can be fit into
> - * DIMM slots.
> - */
>   half_pa = (u64)l1tf_pfn_limit() << PAGE_SHIFT;
>   if (e820__mapped_any(half_pa, ULLONG_MAX - half_pa, E820_TYPE_RAM)) {
>   pr_warn("System has more than MAX_PA/2 memory. L1TF mitigation not effective.\n");
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment