* [PATCH v2] KVM: x86: fix overlap between SPTE_MMIO_MASK and generation
@ 2020-01-23 8:50 Paolo Bonzini
2020-01-23 17:47 ` Ben Gardon
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Bonzini @ 2020-01-23 8:50 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Ben Gardon, Sean Christopherson, stable
The SPTE_MMIO_MASK overlaps with the bits used to track MMIO
generation number. A high enough generation number would overwrite the
SPTE_SPECIAL_MASK region and cause the MMIO SPTE to be misinterpreted.
Likewise, setting bits 52 and 53 would also cause an incorrect generation
number to be read from the PTE, though this was partially mitigated by the
(useless if it weren't for the bug) removal of SPTE_SPECIAL_MASK from
the spte in get_mmio_spte_generation. Drop that removal, and replace
it with a compile-time assertion.
Fixes: 6eeb4ef049e7 ("KVM: x86: assign two bits to track SPTE kinds")
Reported-by: Ben Gardon <bgardon@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/mmu/mmu.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 57e4dbddba72..b9052c7ba43d 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -418,22 +418,24 @@ static inline bool is_access_track_spte(u64 spte)
* requires a full MMU zap). The flag is instead explicitly queried when
* checking for MMIO spte cache hits.
*/
-#define MMIO_SPTE_GEN_MASK GENMASK_ULL(18, 0)
+#define MMIO_SPTE_GEN_MASK GENMASK_ULL(17, 0)
#define MMIO_SPTE_GEN_LOW_START 3
#define MMIO_SPTE_GEN_LOW_END 11
#define MMIO_SPTE_GEN_LOW_MASK GENMASK_ULL(MMIO_SPTE_GEN_LOW_END, \
MMIO_SPTE_GEN_LOW_START)
-#define MMIO_SPTE_GEN_HIGH_START 52
-#define MMIO_SPTE_GEN_HIGH_END 61
+#define MMIO_SPTE_GEN_HIGH_START PT64_SECOND_AVAIL_BITS_SHIFT
+#define MMIO_SPTE_GEN_HIGH_END 62
#define MMIO_SPTE_GEN_HIGH_MASK GENMASK_ULL(MMIO_SPTE_GEN_HIGH_END, \
MMIO_SPTE_GEN_HIGH_START)
+
static u64 generation_mmio_spte_mask(u64 gen)
{
u64 mask;
WARN_ON(gen & ~MMIO_SPTE_GEN_MASK);
+ BUILD_BUG_ON((MMIO_SPTE_GEN_HIGH_MASK | MMIO_SPTE_GEN_LOW_MASK) & SPTE_SPECIAL_MASK);
mask = (gen << MMIO_SPTE_GEN_LOW_START) & MMIO_SPTE_GEN_LOW_MASK;
mask |= (gen << MMIO_SPTE_GEN_HIGH_START) & MMIO_SPTE_GEN_HIGH_MASK;
@@ -444,8 +446,6 @@ static u64 get_mmio_spte_generation(u64 spte)
{
u64 gen;
- spte &= ~shadow_mmio_mask;
-
gen = (spte & MMIO_SPTE_GEN_LOW_MASK) >> MMIO_SPTE_GEN_LOW_START;
gen |= (spte & MMIO_SPTE_GEN_HIGH_MASK) >> MMIO_SPTE_GEN_HIGH_START;
return gen;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] KVM: x86: fix overlap between SPTE_MMIO_MASK and generation
2020-01-23 8:50 [PATCH v2] KVM: x86: fix overlap between SPTE_MMIO_MASK and generation Paolo Bonzini
@ 2020-01-23 17:47 ` Ben Gardon
0 siblings, 0 replies; 2+ messages in thread
From: Ben Gardon @ 2020-01-23 17:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, Sean Christopherson, stable
On Thu, Jan 23, 2020 at 12:50 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The SPTE_MMIO_MASK overlaps with the bits used to track MMIO
> generation number. A high enough generation number would overwrite the
> SPTE_SPECIAL_MASK region and cause the MMIO SPTE to be misinterpreted.
>
> Likewise, setting bits 52 and 53 would also cause an incorrect generation
> number to be read from the PTE, though this was partially mitigated by the
> (useless if it weren't for the bug) removal of SPTE_SPECIAL_MASK from
> the spte in get_mmio_spte_generation. Drop that removal, and replace
> it with a compile-time assertion.
>
> Fixes: 6eeb4ef049e7 ("KVM: x86: assign two bits to track SPTE kinds")
> Reported-by: Ben Gardon <bgardon@google.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 57e4dbddba72..b9052c7ba43d 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -418,22 +418,24 @@ static inline bool is_access_track_spte(u64 spte)
> * requires a full MMU zap). The flag is instead explicitly queried when
> * checking for MMIO spte cache hits.
> */
> -#define MMIO_SPTE_GEN_MASK GENMASK_ULL(18, 0)
> +#define MMIO_SPTE_GEN_MASK GENMASK_ULL(17, 0)
>
> #define MMIO_SPTE_GEN_LOW_START 3
> #define MMIO_SPTE_GEN_LOW_END 11
> #define MMIO_SPTE_GEN_LOW_MASK GENMASK_ULL(MMIO_SPTE_GEN_LOW_END, \
> MMIO_SPTE_GEN_LOW_START)
>
> -#define MMIO_SPTE_GEN_HIGH_START 52
> -#define MMIO_SPTE_GEN_HIGH_END 61
> +#define MMIO_SPTE_GEN_HIGH_START PT64_SECOND_AVAIL_BITS_SHIFT
> +#define MMIO_SPTE_GEN_HIGH_END 62
> #define MMIO_SPTE_GEN_HIGH_MASK GENMASK_ULL(MMIO_SPTE_GEN_HIGH_END, \
> MMIO_SPTE_GEN_HIGH_START)
> +
> static u64 generation_mmio_spte_mask(u64 gen)
> {
> u64 mask;
>
> WARN_ON(gen & ~MMIO_SPTE_GEN_MASK);
> + BUILD_BUG_ON((MMIO_SPTE_GEN_HIGH_MASK | MMIO_SPTE_GEN_LOW_MASK) & SPTE_SPECIAL_MASK);
>
> mask = (gen << MMIO_SPTE_GEN_LOW_START) & MMIO_SPTE_GEN_LOW_MASK;
> mask |= (gen << MMIO_SPTE_GEN_HIGH_START) & MMIO_SPTE_GEN_HIGH_MASK;
> @@ -444,8 +446,6 @@ static u64 get_mmio_spte_generation(u64 spte)
> {
> u64 gen;
>
> - spte &= ~shadow_mmio_mask;
> -
> gen = (spte & MMIO_SPTE_GEN_LOW_MASK) >> MMIO_SPTE_GEN_LOW_START;
> gen |= (spte & MMIO_SPTE_GEN_HIGH_MASK) >> MMIO_SPTE_GEN_HIGH_START;
> return gen;
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-01-23 17:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 8:50 [PATCH v2] KVM: x86: fix overlap between SPTE_MMIO_MASK and generation Paolo Bonzini
2020-01-23 17:47 ` Ben Gardon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).