Message ID | 20181017181215.5841-1-cascardo@canonical.com |
---|---|
State | New |
Headers | show |
Series | [SRU,Cosmic] KVM: x86: fix L1TF's MMIO GFN calculation | expand |
Acked-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
On 17.10.18 20:12, Thadeu Lima de Souza Cascardo wrote: > From: Sean Christopherson <sean.j.christopherson@intel.com> > > BugLink: https://bugs.launchpad.net/bugs/1798427 > > One defense against L1TF in KVM is to always set the upper five bits > of the *legal* physical address in the SPTEs for non-present and > reserved SPTEs, e.g. MMIO SPTEs. In the MMIO case, the GFN of the > MMIO SPTE may overlap with the upper five bits that are being usurped > to defend against L1TF. To preserve the GFN, the bits of the GFN that > overlap with the repurposed bits are shifted left into the reserved > bits, i.e. the GFN in the SPTE will be split into high and low parts. > When retrieving the GFN from the MMIO SPTE, e.g. to check for an MMIO > access, get_mmio_spte_gfn() unshifts the affected bits and restores > the original GFN for comparison. Unfortunately, get_mmio_spte_gfn() > neglects to mask off the reserved bits in the SPTE that were used to > store the upper chunk of the GFN. As a result, KVM fails to detect > MMIO accesses whose GPA overlaps the repurprosed bits, which in turn > causes guest panics and hangs. > > Fix the bug by generating a mask that covers the lower chunk of the > GFN, i.e. the bits that aren't shifted by the L1TF mitigation. The > alternative approach would be to explicitly zero the five reserved > bits that are used to store the upper chunk of the GFN, but that > requires additional run-time computation and makes an already-ugly > bit of code even more inscrutable. > > I considered adding a WARN_ON_ONCE(low_phys_bits-1 <= PAGE_SHIFT) to > warn if GENMASK_ULL() generated a nonsensical value, but that seemed > silly since that would mean a system that supports VMX has less than > 18 bits of physical address space... > > Reported-by: Sakari Ailus <sakari.ailus@iki.fi> > Fixes: d9b47449c1a1 ("kvm: x86: Set highest physical address bits in non-present/reserved SPTEs") > Cc: Junaid Shahid <junaids@google.com> > Cc: Jim Mattson <jmattson@google.com> > Cc: stable@vger.kernel.org > Reviewed-by: Junaid Shahid <junaids@google.com> > Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (cherry picked from commit daa07cbc9ae3da2d61b7ce900c0b9107d134f2c1) > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > arch/x86/kvm/mmu.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 97d41754769e..d02f0390c1c1 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -232,6 +232,17 @@ static u64 __read_mostly shadow_nonpresent_or_rsvd_mask; > */ > static const u64 shadow_nonpresent_or_rsvd_mask_len = 5; > > +/* > + * In some cases, we need to preserve the GFN of a non-present or reserved > + * SPTE when we usurp the upper five bits of the physical address space to > + * defend against L1TF, e.g. for MMIO SPTEs. To preserve the GFN, we'll > + * shift bits of the GFN that overlap with shadow_nonpresent_or_rsvd_mask > + * left into the reserved bits, i.e. the GFN in the SPTE will be split into > + * high and low parts. This mask covers the lower bits of the GFN. > + */ > +static u64 __read_mostly shadow_nonpresent_or_rsvd_lower_gfn_mask; > + > + > static void mmu_spte_set(u64 *sptep, u64 spte); > > void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value) > @@ -338,9 +349,7 @@ static bool is_mmio_spte(u64 spte) > > static gfn_t get_mmio_spte_gfn(u64 spte) > { > - u64 mask = generation_mmio_spte_mask(MMIO_GEN_MASK) | shadow_mmio_mask | > - shadow_nonpresent_or_rsvd_mask; > - u64 gpa = spte & ~mask; > + u64 gpa = spte & shadow_nonpresent_or_rsvd_lower_gfn_mask; > > gpa |= (spte >> shadow_nonpresent_or_rsvd_mask_len) > & shadow_nonpresent_or_rsvd_mask; > @@ -404,6 +413,8 @@ EXPORT_SYMBOL_GPL(kvm_mmu_set_mask_ptes); > > static void kvm_mmu_reset_all_pte_masks(void) > { > + u8 low_phys_bits; > + > shadow_user_mask = 0; > shadow_accessed_mask = 0; > shadow_dirty_mask = 0; > @@ -418,12 +429,17 @@ static void kvm_mmu_reset_all_pte_masks(void) > * appropriate mask to guard against L1TF attacks. Otherwise, it is > * assumed that the CPU is not vulnerable to L1TF. > */ > + low_phys_bits = boot_cpu_data.x86_phys_bits; > if (boot_cpu_data.x86_phys_bits < > - 52 - shadow_nonpresent_or_rsvd_mask_len) > + 52 - shadow_nonpresent_or_rsvd_mask_len) { > shadow_nonpresent_or_rsvd_mask = > rsvd_bits(boot_cpu_data.x86_phys_bits - > shadow_nonpresent_or_rsvd_mask_len, > boot_cpu_data.x86_phys_bits - 1); > + low_phys_bits -= shadow_nonpresent_or_rsvd_mask_len; > + } > + shadow_nonpresent_or_rsvd_lower_gfn_mask = > + GENMASK_ULL(low_phys_bits - 1, PAGE_SHIFT); > } > > static int is_cpuid_PSE36(void) >
On 2018-10-17 15:12:15 , Thadeu Lima de Souza Cascardo wrote: > From: Sean Christopherson <sean.j.christopherson@intel.com> > > BugLink: https://bugs.launchpad.net/bugs/1798427 > > One defense against L1TF in KVM is to always set the upper five bits > of the *legal* physical address in the SPTEs for non-present and > reserved SPTEs, e.g. MMIO SPTEs. In the MMIO case, the GFN of the > MMIO SPTE may overlap with the upper five bits that are being usurped > to defend against L1TF. To preserve the GFN, the bits of the GFN that > overlap with the repurposed bits are shifted left into the reserved > bits, i.e. the GFN in the SPTE will be split into high and low parts. > When retrieving the GFN from the MMIO SPTE, e.g. to check for an MMIO > access, get_mmio_spte_gfn() unshifts the affected bits and restores > the original GFN for comparison. Unfortunately, get_mmio_spte_gfn() > neglects to mask off the reserved bits in the SPTE that were used to > store the upper chunk of the GFN. As a result, KVM fails to detect > MMIO accesses whose GPA overlaps the repurprosed bits, which in turn > causes guest panics and hangs. > > Fix the bug by generating a mask that covers the lower chunk of the > GFN, i.e. the bits that aren't shifted by the L1TF mitigation. The > alternative approach would be to explicitly zero the five reserved > bits that are used to store the upper chunk of the GFN, but that > requires additional run-time computation and makes an already-ugly > bit of code even more inscrutable. > > I considered adding a WARN_ON_ONCE(low_phys_bits-1 <= PAGE_SHIFT) to > warn if GENMASK_ULL() generated a nonsensical value, but that seemed > silly since that would mean a system that supports VMX has less than > 18 bits of physical address space... > > Reported-by: Sakari Ailus <sakari.ailus@iki.fi> > Fixes: d9b47449c1a1 ("kvm: x86: Set highest physical address bits in non-present/reserved SPTEs") > Cc: Junaid Shahid <junaids@google.com> > Cc: Jim Mattson <jmattson@google.com> > Cc: stable@vger.kernel.org > Reviewed-by: Junaid Shahid <junaids@google.com> > Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (cherry picked from commit daa07cbc9ae3da2d61b7ce900c0b9107d134f2c1) > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > arch/x86/kvm/mmu.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 97d41754769e..d02f0390c1c1 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -232,6 +232,17 @@ static u64 __read_mostly shadow_nonpresent_or_rsvd_mask; > */ > static const u64 shadow_nonpresent_or_rsvd_mask_len = 5; > > +/* > + * In some cases, we need to preserve the GFN of a non-present or reserved > + * SPTE when we usurp the upper five bits of the physical address space to > + * defend against L1TF, e.g. for MMIO SPTEs. To preserve the GFN, we'll > + * shift bits of the GFN that overlap with shadow_nonpresent_or_rsvd_mask > + * left into the reserved bits, i.e. the GFN in the SPTE will be split into > + * high and low parts. This mask covers the lower bits of the GFN. > + */ > +static u64 __read_mostly shadow_nonpresent_or_rsvd_lower_gfn_mask; > + > + > static void mmu_spte_set(u64 *sptep, u64 spte); > > void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value) > @@ -338,9 +349,7 @@ static bool is_mmio_spte(u64 spte) > > static gfn_t get_mmio_spte_gfn(u64 spte) > { > - u64 mask = generation_mmio_spte_mask(MMIO_GEN_MASK) | shadow_mmio_mask | > - shadow_nonpresent_or_rsvd_mask; > - u64 gpa = spte & ~mask; > + u64 gpa = spte & shadow_nonpresent_or_rsvd_lower_gfn_mask; > > gpa |= (spte >> shadow_nonpresent_or_rsvd_mask_len) > & shadow_nonpresent_or_rsvd_mask; > @@ -404,6 +413,8 @@ EXPORT_SYMBOL_GPL(kvm_mmu_set_mask_ptes); > > static void kvm_mmu_reset_all_pte_masks(void) > { > + u8 low_phys_bits; > + > shadow_user_mask = 0; > shadow_accessed_mask = 0; > shadow_dirty_mask = 0; > @@ -418,12 +429,17 @@ static void kvm_mmu_reset_all_pte_masks(void) > * appropriate mask to guard against L1TF attacks. Otherwise, it is > * assumed that the CPU is not vulnerable to L1TF. > */ > + low_phys_bits = boot_cpu_data.x86_phys_bits; > if (boot_cpu_data.x86_phys_bits < > - 52 - shadow_nonpresent_or_rsvd_mask_len) > + 52 - shadow_nonpresent_or_rsvd_mask_len) { > shadow_nonpresent_or_rsvd_mask = > rsvd_bits(boot_cpu_data.x86_phys_bits - > shadow_nonpresent_or_rsvd_mask_len, > boot_cpu_data.x86_phys_bits - 1); > + low_phys_bits -= shadow_nonpresent_or_rsvd_mask_len; > + } > + shadow_nonpresent_or_rsvd_lower_gfn_mask = > + GENMASK_ULL(low_phys_bits - 1, PAGE_SHIFT); > } > > static int is_cpuid_PSE36(void) > -- > 2.19.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 97d41754769e..d02f0390c1c1 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -232,6 +232,17 @@ static u64 __read_mostly shadow_nonpresent_or_rsvd_mask; */ static const u64 shadow_nonpresent_or_rsvd_mask_len = 5; +/* + * In some cases, we need to preserve the GFN of a non-present or reserved + * SPTE when we usurp the upper five bits of the physical address space to + * defend against L1TF, e.g. for MMIO SPTEs. To preserve the GFN, we'll + * shift bits of the GFN that overlap with shadow_nonpresent_or_rsvd_mask + * left into the reserved bits, i.e. the GFN in the SPTE will be split into + * high and low parts. This mask covers the lower bits of the GFN. + */ +static u64 __read_mostly shadow_nonpresent_or_rsvd_lower_gfn_mask; + + static void mmu_spte_set(u64 *sptep, u64 spte); void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value) @@ -338,9 +349,7 @@ static bool is_mmio_spte(u64 spte) static gfn_t get_mmio_spte_gfn(u64 spte) { - u64 mask = generation_mmio_spte_mask(MMIO_GEN_MASK) | shadow_mmio_mask | - shadow_nonpresent_or_rsvd_mask; - u64 gpa = spte & ~mask; + u64 gpa = spte & shadow_nonpresent_or_rsvd_lower_gfn_mask; gpa |= (spte >> shadow_nonpresent_or_rsvd_mask_len) & shadow_nonpresent_or_rsvd_mask; @@ -404,6 +413,8 @@ EXPORT_SYMBOL_GPL(kvm_mmu_set_mask_ptes); static void kvm_mmu_reset_all_pte_masks(void) { + u8 low_phys_bits; + shadow_user_mask = 0; shadow_accessed_mask = 0; shadow_dirty_mask = 0; @@ -418,12 +429,17 @@ static void kvm_mmu_reset_all_pte_masks(void) * appropriate mask to guard against L1TF attacks. Otherwise, it is * assumed that the CPU is not vulnerable to L1TF. */ + low_phys_bits = boot_cpu_data.x86_phys_bits; if (boot_cpu_data.x86_phys_bits < - 52 - shadow_nonpresent_or_rsvd_mask_len) + 52 - shadow_nonpresent_or_rsvd_mask_len) { shadow_nonpresent_or_rsvd_mask = rsvd_bits(boot_cpu_data.x86_phys_bits - shadow_nonpresent_or_rsvd_mask_len, boot_cpu_data.x86_phys_bits - 1); + low_phys_bits -= shadow_nonpresent_or_rsvd_mask_len; + } + shadow_nonpresent_or_rsvd_lower_gfn_mask = + GENMASK_ULL(low_phys_bits - 1, PAGE_SHIFT); } static int is_cpuid_PSE36(void)