KVM: x86/mmu: Allocate the lm_root before allocating PAE roots

Allocate lm_root before the PAE roots so that the PAE roots aren't
leaked if the memory allocation for the lm_root happens to fail.

Note, KVM can still leak PAE roots if mmu_check_root() fails on a guest's
PDPTR, or if mmu_alloc_root() fails due to MMU pages not being available.
Those issues will be fixed in future commits.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-Id: <20210305011101.3597423-5-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 89eec1d..973dcc1 100644 (file)
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3314,21 +3314,38 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
         * the shadow page table may be a PAE or a long mode page table.
         */
        pm_mask = PT_PRESENT_MASK;
-       if (mmu->shadow_root_level == PT64_ROOT_4LEVEL) {
+       if (mmu->shadow_root_level == PT64_ROOT_4LEVEL)
                pm_mask |= PT_ACCESSED_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
 
-               /*
-                * Allocate the page for the PDPTEs when shadowing 32-bit NPT
-                * with 64-bit only when needed.  Unlike 32-bit NPT, it doesn't
-                * need to be in low mem.  See also lm_root below.
-                */
-               if (!mmu->pae_root) {
-                       WARN_ON_ONCE(!tdp_enabled);
+       /*
+        * When shadowing 32-bit or PAE NPT with 64-bit NPT, the PML4 and PDP
+        * tables are allocated and initialized at root creation as there is no
+        * equivalent level in the guest's NPT to shadow.  Allocate the tables
+        * on demand, as running a 32-bit L1 VMM is very rare.  Unlike 32-bit
+        * NPT, the PDP table doesn't need to be in low mem.  Preallocate the
+        * pages so that the PAE roots aren't leaked on failure.
+        */
+       if (mmu->shadow_root_level == PT64_ROOT_4LEVEL &&
+           (!mmu->pae_root || !mmu->lm_root)) {
+               u64 *lm_root, *pae_root;
 
-                       mmu->pae_root = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
-                       if (!mmu->pae_root)
-                               return -ENOMEM;
+               if (WARN_ON_ONCE(!tdp_enabled || mmu->pae_root || mmu->lm_root))
+                       return -EIO;
+
+               pae_root = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+               if (!pae_root)
+                       return -ENOMEM;
+
+               lm_root = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+               if (!lm_root) {
+                       free_page((unsigned long)pae_root);
+                       return -ENOMEM;
                }
+
+               mmu->pae_root = pae_root;
+               mmu->lm_root = lm_root;
+
+               lm_root[0] = __pa(mmu->pae_root) | pm_mask;
        }
 
        for (i = 0; i < 4; ++i) {
@@ -3350,30 +3367,11 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
                        return -ENOSPC;
                mmu->pae_root[i] = root | pm_mask;
        }
-       mmu->root_hpa = __pa(mmu->pae_root);
-
-       /*
-        * When shadowing 32-bit or PAE NPT with 64-bit NPT, the PML4 and PDP
-        * tables are allocated and initialized at MMU creation as there is no
-        * equivalent level in the guest's NPT to shadow.  Allocate the tables
-        * on demand, as running a 32-bit L1 VMM is very rare.  The PDP is
-        * handled above (to share logic with PAE), deal with the PML4 here.
-        */
-       if (mmu->shadow_root_level == PT64_ROOT_4LEVEL) {
-               if (mmu->lm_root == NULL) {
-                       u64 *lm_root;
-
-                       lm_root = (void*)get_zeroed_page(GFP_KERNEL_ACCOUNT);
-                       if (!lm_root)
-                               return -ENOMEM;
-
-                       lm_root[0] = __pa(mmu->pae_root) | pm_mask;
-
-                       mmu->lm_root = lm_root;
-               }
 
+       if (mmu->shadow_root_level == PT64_ROOT_4LEVEL)
                mmu->root_hpa = __pa(mmu->lm_root);
-       }
+       else
+               mmu->root_hpa = __pa(mmu->pae_root);
 
 set_root_pgd:
        mmu->root_pgd = root_pgd;