[bionic][PATCH 0/3] Patch for CVE-2019-18660

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

[bionic][PATCH 0/3] Patch for CVE-2019-18660

Benjamin M Romer
CVE-2019-18660:

The Linux kernel through 5.3.13 on powerpc allows Information Exposure
because the Spectre-RSB mitigation is not in place for all applicable
CPUs, aka CID-39e72bf96f58. This is related to
arch/powerpc/kernel/entry_64.S and arch/powerpc/kernel/security.c.

Christopher M. Riedl (1):
  powerpc/64s: support nospectre_v2 cmdline option

Michael Ellerman (2):
  powerpc/book3s64: Fix link stack flush on context switch
  KVM: PPC: Book3S HV: Flush link stack on guest exit to host kernel

 arch/powerpc/include/asm/asm-prototypes.h    |  3 +
 arch/powerpc/include/asm/security_features.h |  3 +
 arch/powerpc/kernel/entry_64.S               |  6 ++
 arch/powerpc/kernel/security.c               | 74 ++++++++++++++++++--
 arch/powerpc/kvm/book3s_hv_rmhandlers.S      | 28 ++++++++
 5 files changed, 108 insertions(+), 6 deletions(-)

--
2.20.1


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

[bionic][PATCH 1/3] powerpc/64s: support nospectre_v2 cmdline option

Benjamin M Romer
From: "Christopher M. Riedl" <[hidden email]>

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

commit d8f0e0b073e1ec52a05f0c2a56318b47387d2f10 upstream.

Add support for disabling the kernel implemented spectre v2 mitigation
(count cache flush on context switch) via the nospectre_v2 and
mitigations=off cmdline options.

Suggested-by: Michael Ellerman <[hidden email]>
Signed-off-by: Christopher M. Riedl <[hidden email]>
Reviewed-by: Andrew Donnellan <[hidden email]>
Signed-off-by: Michael Ellerman <[hidden email]>
Link: https://lore.kernel.org/r/20190524024647.381-1-cmr@...
Signed-off-by: Daniel Axtens <[hidden email]>

CVE-2019-18660
Signed-off-by: Benjamin M Romer <[hidden email]>
---
 arch/powerpc/kernel/security.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 70568ccbd9fd..8cc8b0de12d2 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -28,7 +28,7 @@ static enum count_cache_flush_type count_cache_flush_type = COUNT_CACHE_FLUSH_NO
 bool barrier_nospec_enabled;
 static bool no_nospec;
 static bool btb_flush_enabled;
-#ifdef CONFIG_PPC_FSL_BOOK3E
+#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3S_64)
 static bool no_spectrev2;
 #endif
 
@@ -106,7 +106,7 @@ static __init int barrier_nospec_debugfs_init(void)
 device_initcall(barrier_nospec_debugfs_init);
 #endif /* CONFIG_DEBUG_FS */
 
-#ifdef CONFIG_PPC_FSL_BOOK3E
+#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3S_64)
 static int __init handle_nospectre_v2(char *p)
 {
  no_spectrev2 = true;
@@ -114,6 +114,9 @@ static int __init handle_nospectre_v2(char *p)
  return 0;
 }
 early_param("nospectre_v2", handle_nospectre_v2);
+#endif /* CONFIG_PPC_FSL_BOOK3E || CONFIG_PPC_BOOK3S_64 */
+
+#ifdef CONFIG_PPC_FSL_BOOK3E
 void setup_spectre_v2(void)
 {
  if (no_spectrev2 || cpu_mitigations_off())
@@ -391,7 +394,17 @@ static void toggle_count_cache_flush(bool enable)
 
 void setup_count_cache_flush(void)
 {
- toggle_count_cache_flush(true);
+ bool enable = true;
+
+ if (no_spectrev2 || cpu_mitigations_off()) {
+ if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED) ||
+    security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED))
+ pr_warn("Spectre v2 mitigations not under software control, can't disable\n");
+
+ enable = false;
+ }
+
+ toggle_count_cache_flush(enable);
 }
 
 #ifdef CONFIG_DEBUG_FS
--
2.20.1


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

[bionic][PATCH 2/3] powerpc/book3s64: Fix link stack flush on context switch

Benjamin M Romer
In reply to this post by Benjamin M Romer
From: Michael Ellerman <[hidden email]>

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

commit 39e72bf96f5847ba87cc5bd7a3ce0fed813dc9ad upstream.

In commit ee13cb249fab ("powerpc/64s: Add support for software count
cache flush"), I added support for software to flush the count
cache (indirect branch cache) on context switch if firmware told us
that was the required mitigation for Spectre v2.

As part of that code we also added a software flush of the link
stack (return address stack), which protects against Spectre-RSB
between user processes.

That is all correct for CPUs that activate that mitigation, which is
currently Power9 Nimbus DD2.3.

What I got wrong is that on older CPUs, where firmware has disabled
the count cache, we also need to flush the link stack on context
switch.

To fix it we create a new feature bit which is not set by firmware,
which tells us we need to flush the link stack. We set that when
firmware tells us that either of the existing Spectre v2 mitigations
are enabled.

Then we adjust the patching code so that if we see that feature bit we
enable the link stack flush. If we're also told to flush the count
cache in software then we fall through and do that also.

On the older CPUs we don't need to do do the software count cache
flush, firmware has disabled it, so in that case we patch in an early
return after the link stack flush.

The naming of some of the functions is awkward after this patch,
because they're called "count cache" but they also do link stack. But
we'll fix that up in a later commit to ease backporting.

This is the fix for CVE-2019-18660.

Reported-by: Anthony Steinhauser <[hidden email]>
Fixes: ee13cb249fab ("powerpc/64s: Add support for software count cache flush")
Cc: [hidden email] # v4.4+
Signed-off-by: Michael Ellerman <[hidden email]>
[dja: backport to Bionic's v4.15]
Signed-off-by: Daniel Axtens <[hidden email]>

CVE-2019-18660
Signed-off-by: Benjamin M Romer <[hidden email]>
---
 arch/powerpc/include/asm/asm-prototypes.h    |  1 +
 arch/powerpc/include/asm/security_features.h |  3 ++
 arch/powerpc/kernel/entry_64.S               |  6 +++
 arch/powerpc/kernel/security.c               | 48 ++++++++++++++++++--
 4 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index b98899c42729..0cb97d9fb003 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -141,6 +141,7 @@ void _kvmppc_save_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
 /* Patch sites */
 extern s32 patch__call_flush_count_cache;
 extern s32 patch__flush_count_cache_return;
+extern s32 patch__flush_link_stack_return;
 
 extern long flush_count_cache;
 
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index 759597bf0fd8..ccf44c135389 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -81,6 +81,9 @@ static inline bool security_ftr_enabled(unsigned long feature)
 // Software required to flush count cache on context switch
 #define SEC_FTR_FLUSH_COUNT_CACHE 0x0000000000000400ull
 
+// Software required to flush link stack on context switch
+#define SEC_FTR_FLUSH_LINK_STACK 0x0000000000001000ull
+
 
 // Features enabled by default
 #define SEC_FTR_DEFAULT \
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 5e507c1e07cd..44ab62c28bf2 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -524,6 +524,7 @@ flush_count_cache:
  /* Save LR into r9 */
  mflr r9
 
+ // Flush the link stack
  .rept 64
  bl .+4
  .endr
@@ -533,6 +534,11 @@ flush_count_cache:
  .balign 32
  /* Restore LR */
 1: mtlr r9
+
+ // If we're just flushing the link stack, return here
+3: nop
+ patch_site 3b patch__flush_link_stack_return
+
  li r9,0x7fff
  mtctr r9
 
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 8cc8b0de12d2..6e54a25f398b 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -24,6 +24,7 @@ enum count_cache_flush_type {
  COUNT_CACHE_FLUSH_HW = 0x4,
 };
 static enum count_cache_flush_type count_cache_flush_type = COUNT_CACHE_FLUSH_NONE;
+static bool link_stack_flush_enabled;
 
 bool barrier_nospec_enabled;
 static bool no_nospec;
@@ -204,11 +205,19 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, c
 
  if (ccd)
  seq_buf_printf(&s, "Indirect branch cache disabled");
+
+ if (link_stack_flush_enabled)
+ seq_buf_printf(&s, ", Software link stack flush");
+
  } else if (count_cache_flush_type != COUNT_CACHE_FLUSH_NONE) {
  seq_buf_printf(&s, "Mitigation: Software count cache flush");
 
  if (count_cache_flush_type == COUNT_CACHE_FLUSH_HW)
  seq_buf_printf(&s, " (hardware accelerated)");
+
+ if (link_stack_flush_enabled)
+ seq_buf_printf(&s, ", Software link stack flush");
+
  } else if (btb_flush_enabled) {
  seq_buf_printf(&s, "Mitigation: Branch predictor state flush");
  } else {
@@ -369,18 +378,40 @@ static __init int stf_barrier_debugfs_init(void)
 device_initcall(stf_barrier_debugfs_init);
 #endif /* CONFIG_DEBUG_FS */
 
+static void no_count_cache_flush(void)
+{
+ count_cache_flush_type = COUNT_CACHE_FLUSH_NONE;
+ pr_info("count-cache-flush: software flush disabled.\n");
+}
+
 static void toggle_count_cache_flush(bool enable)
 {
- if (!enable || !security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
+ if (!security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE) &&
+    !security_ftr_enabled(SEC_FTR_FLUSH_LINK_STACK))
+ enable = false;
+
+ if (!enable) {
  patch_instruction_site(&patch__call_flush_count_cache, PPC_INST_NOP);
- count_cache_flush_type = COUNT_CACHE_FLUSH_NONE;
- pr_info("count-cache-flush: software flush disabled.\n");
+ pr_info("link-stack-flush: software flush disabled.\n");
+ link_stack_flush_enabled = false;
+ no_count_cache_flush();
  return;
  }
 
+ // This enables the branch from _switch to flush_count_cache
  patch_branch_site(&patch__call_flush_count_cache,
   (u64)&flush_count_cache, BRANCH_SET_LINK);
 
+ pr_info("link-stack-flush: software flush enabled.\n");
+ link_stack_flush_enabled = true;
+
+ // If we just need to flush the link stack, patch an early return
+ if (!security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
+ patch_instruction_site(&patch__flush_link_stack_return, PPC_INST_BLR);
+ no_count_cache_flush();
+ return;
+ }
+
  if (!security_ftr_enabled(SEC_FTR_BCCTR_FLUSH_ASSIST)) {
  count_cache_flush_type = COUNT_CACHE_FLUSH_SW;
  pr_info("count-cache-flush: full software flush sequence enabled.\n");
@@ -399,11 +430,20 @@ void setup_count_cache_flush(void)
  if (no_spectrev2 || cpu_mitigations_off()) {
  if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED) ||
     security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED))
- pr_warn("Spectre v2 mitigations not under software control, can't disable\n");
+ pr_warn("Spectre v2 mitigations not fully under software control, can't disable\n");
 
  enable = false;
  }
 
+ /*
+ * There's no firmware feature flag/hypervisor bit to tell us we need to
+ * flush the link stack on context switch. So we set it here if we see
+ * either of the Spectre v2 mitigations that aim to protect userspace.
+ */
+ if (security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED) ||
+    security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE))
+ security_ftr_set(SEC_FTR_FLUSH_LINK_STACK);
+
  toggle_count_cache_flush(enable);
 }
 
--
2.20.1


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

[bionic][PATCH 3/3] KVM: PPC: Book3S HV: Flush link stack on guest exit to host kernel

Benjamin M Romer
In reply to this post by Benjamin M Romer
From: Michael Ellerman <[hidden email]>

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

commit af2e8c68b9c5403f77096969c516f742f5bb29e0 upstream.

On some systems that are vulnerable to Spectre v2, it is up to
software to flush the link stack (return address stack), in order to
protect against Spectre-RSB.

When exiting from a guest we do some house keeping and then
potentially exit to C code which is several stack frames deep in the
host kernel. We will then execute a series of returns without
preceeding calls, opening up the possiblity that the guest could have
poisoned the link stack, and direct speculative execution of the host
to a gadget of some sort.

To prevent this we add a flush of the link stack on exit from a guest.

Signed-off-by: Michael Ellerman <[hidden email]>
[dja: backport to Bionic's v4.15]
Signed-off-by: Daniel Axtens <[hidden email]>

CVE-2019-18660
Signed-off-by: Benjamin M Romer <[hidden email]>
---
 arch/powerpc/include/asm/asm-prototypes.h |  2 ++
 arch/powerpc/kernel/security.c            |  9 ++++++++
 arch/powerpc/kvm/book3s_hv_rmhandlers.S   | 28 +++++++++++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 0cb97d9fb003..c363d747e9bb 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -142,7 +142,9 @@ void _kvmppc_save_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
 extern s32 patch__call_flush_count_cache;
 extern s32 patch__flush_count_cache_return;
 extern s32 patch__flush_link_stack_return;
+extern s32 patch__call_kvm_flush_link_stack;
 
 extern long flush_count_cache;
+extern long kvm_flush_link_stack;
 
 #endif /* _ASM_POWERPC_ASM_PROTOTYPES_H */
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 6e54a25f398b..a5c5940d970a 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -392,6 +392,9 @@ static void toggle_count_cache_flush(bool enable)
 
  if (!enable) {
  patch_instruction_site(&patch__call_flush_count_cache, PPC_INST_NOP);
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ patch_instruction_site(&patch__call_kvm_flush_link_stack, PPC_INST_NOP);
+#endif
  pr_info("link-stack-flush: software flush disabled.\n");
  link_stack_flush_enabled = false;
  no_count_cache_flush();
@@ -402,6 +405,12 @@ static void toggle_count_cache_flush(bool enable)
  patch_branch_site(&patch__call_flush_count_cache,
   (u64)&flush_count_cache, BRANCH_SET_LINK);
 
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ // This enables the branch from guest_exit_cont to kvm_flush_link_stack
+ patch_branch_site(&patch__call_kvm_flush_link_stack,
+  (u64)&kvm_flush_link_stack, BRANCH_SET_LINK);
+#endif
+
  pr_info("link-stack-flush: software flush enabled.\n");
  link_stack_flush_enabled = true;
 
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 80c49b0306f7..b5bd2f4902b3 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -18,6 +18,7 @@
  */
 
 #include <asm/ppc_asm.h>
+#include <asm/code-patching-asm.h>
 #include <asm/kvm_asm.h>
 #include <asm/reg.h>
 #include <asm/mmu.h>
@@ -1506,6 +1507,10 @@ mc_cont:
 1:
 #endif /* CONFIG_KVM_XICS */
 
+ /* Possibly flush the link stack here. */
+1: nop
+ patch_site 1b patch__call_kvm_flush_link_stack
+
  /* For hash guest, read the guest SLB and save it away */
  ld r5, VCPU_KVM(r9)
  lbz r0, KVM_RADIX(r5)
@@ -2039,6 +2044,29 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
  mtlr r0
  blr
 
+.balign 32
+.global kvm_flush_link_stack
+kvm_flush_link_stack:
+ /* Save LR into r0 */
+ mflr r0
+
+ /* Flush the link stack. On Power8 it's up to 32 entries in size. */
+ .rept 32
+ bl .+4
+ .endr
+
+ /* And on Power9 it's up to 64. */
+BEGIN_FTR_SECTION
+ .rept 32
+ bl .+4
+ .endr
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
+
+ /* Restore LR */
+ mtlr r0
+ blr
+
+
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 /*
  * Softpatch interrupt for transactional memory emulation cases
--
2.20.1


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

ACK: [bionic][PATCH 0/3] Patch for CVE-2019-18660

Stefan Bader-2
In reply to this post by Benjamin M Romer
On 28.11.19 16:05, Benjamin M Romer wrote:

> CVE-2019-18660:
>
> The Linux kernel through 5.3.13 on powerpc allows Information Exposure
> because the Spectre-RSB mitigation is not in place for all applicable
> CPUs, aka CID-39e72bf96f58. This is related to
> arch/powerpc/kernel/entry_64.S and arch/powerpc/kernel/security.c.
>
> Christopher M. Riedl (1):
>   powerpc/64s: support nospectre_v2 cmdline option
>
> Michael Ellerman (2):
>   powerpc/book3s64: Fix link stack flush on context switch
>   KVM: PPC: Book3S HV: Flush link stack on guest exit to host kernel
>
>  arch/powerpc/include/asm/asm-prototypes.h    |  3 +
>  arch/powerpc/include/asm/security_features.h |  3 +
>  arch/powerpc/kernel/entry_64.S               |  6 ++
>  arch/powerpc/kernel/security.c               | 74 ++++++++++++++++++--
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S      | 28 ++++++++
>  5 files changed, 108 insertions(+), 6 deletions(-)
>
Acked-by: Stefan Bader <[hidden email]>


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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [bionic][PATCH 0/3] Patch for CVE-2019-18660

Kleber Souza
In reply to this post by Benjamin M Romer
On 2019-11-28 16:05, Benjamin M Romer wrote:

> CVE-2019-18660:
>
> The Linux kernel through 5.3.13 on powerpc allows Information Exposure
> because the Spectre-RSB mitigation is not in place for all applicable
> CPUs, aka CID-39e72bf96f58. This is related to
> arch/powerpc/kernel/entry_64.S and arch/powerpc/kernel/security.c.
>
> Christopher M. Riedl (1):
>   powerpc/64s: support nospectre_v2 cmdline option
>
> Michael Ellerman (2):
>   powerpc/book3s64: Fix link stack flush on context switch
>   KVM: PPC: Book3S HV: Flush link stack on guest exit to host kernel
>
>  arch/powerpc/include/asm/asm-prototypes.h    |  3 +
>  arch/powerpc/include/asm/security_features.h |  3 +
>  arch/powerpc/kernel/entry_64.S               |  6 ++
>  arch/powerpc/kernel/security.c               | 74 ++++++++++++++++++--
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S      | 28 ++++++++
>  5 files changed, 108 insertions(+), 6 deletions(-)
>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

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

APPLIED: [bionic][PATCH 0/3] Patch for CVE-2019-18660

Kleber Souza
In reply to this post by Benjamin M Romer
On 2019-11-28 16:05, Benjamin M Romer wrote:

> CVE-2019-18660:
>
> The Linux kernel through 5.3.13 on powerpc allows Information Exposure
> because the Spectre-RSB mitigation is not in place for all applicable
> CPUs, aka CID-39e72bf96f58. This is related to
> arch/powerpc/kernel/entry_64.S and arch/powerpc/kernel/security.c.
>
> Christopher M. Riedl (1):
>   powerpc/64s: support nospectre_v2 cmdline option
>
> Michael Ellerman (2):
>   powerpc/book3s64: Fix link stack flush on context switch
>   KVM: PPC: Book3S HV: Flush link stack on guest exit to host kernel
>
>  arch/powerpc/include/asm/asm-prototypes.h    |  3 +
>  arch/powerpc/include/asm/security_features.h |  3 +
>  arch/powerpc/kernel/entry_64.S               |  6 ++
>  arch/powerpc/kernel/security.c               | 74 ++++++++++++++++++--
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S      | 28 ++++++++
>  5 files changed, 108 insertions(+), 6 deletions(-)
>

Applied to bionic/master-next branch.

Thanks,
Kleber

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