KVM: x86: Fix shadow paging use-after-free due to unexpected role

jira VULN-191426
cve CVE-2026-53359
commit-author Paolo Bonzini <pbonzini@redhat.com>
commit 81ccda3
upstream-diff |
	Upstream adds the check to kvm_mmu_get_child_sp(), which doesn't
	exist here, so it goes into kvm_mmu_child_sp_exists() at the reuse
	sites in __direct_map() and FNAME(fetch). The expected role comes
	from kvm_mmu_child_role(), split out of kvm_mmu_get_page() (as
	upstream did in 2e65e84) so the check and the allocator can't
	derive different roles.

Commit 0cb2af2 ("KVM: x86: Fix shadow paging use-after-free due
to unexpected GFN") fixed a shadow paging mismatch between stored and
computed GFNs; the bug could be triggered by changing a PDE mapping from
outside the guest, and then deleting a memslot.  The rmap_remove()
call would miss entries created after the PDE change because the GFN
of the leaf SPTE does not match the GFN of the struct kvm_mmu_page.

A similar hole however remains if the modified PDE points to a non-leaf
page.  In this case the gfn can be made to match, but the role does not
match: the original large 2MB page creates a kvm_mmu_page with direct=1,
while the new 4KB needs a kvm_mmu_page with direct=0.  However,
kvm_mmu_get_child_sp() does not compare the role, and therefore reuses
the page.

The next step is installing a leaf (4KB) SPTE on the new path which
records an rmap entry under the gfn resolved by the walk.  But when
that child is zapped its parent kvm_mmu_page has direct=1 and
kvm_mmu_page_get_gfn() computes the gfn for the 4KB page as
sp->gfn + index instead of using sp->shadowed_translation[] (or sp->gfns[]
in older kernels).  It therefore fails to remove the recorded entry.

When the memslot is dropped the shadow page is freed but the rmap
entry survives, as in the scenario that was already fixed.  Code that
later walks that gfn (dirty logging, MMU notifier invalidation, and
so on) dereferences an sptep that lies in the freed page, causing the
use-after-free.

Fixes: 2032a93 ("KVM: MMU: Don't allocate gfns page for direct mmu pages")
	Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
	Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 81ccda3)
	Signed-off-by: Sultan Alsawaf <sultan@ciq.com>

Author: kerneltoast

arch/x86/kvm/mmu/mmu.c

@@ -2053,20 +2053,13 @@ static void clear_sp_write_flooding_count(u64 *spte)
 	__clear_sp_write_flooding_count(sptep_to_sp(spte));
 }
 
-static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
-					     gfn_t gfn,
-					     gva_t gaddr,
-					     unsigned level,
-					     int direct,
-					     unsigned int access)
+static union kvm_mmu_page_role
+kvm_mmu_child_role(struct kvm_vcpu *vcpu, gva_t gaddr, unsigned level,
+		   int direct, unsigned int access)
 {
 	bool direct_mmu = vcpu->arch.mmu->direct_map;
 	union kvm_mmu_page_role role;
-	struct hlist_head *sp_list;
 	unsigned quadrant;
-	struct kvm_mmu_page *sp;
-	int collisions = 0;
-	LIST_HEAD(invalid_list);
 
 	role = vcpu->arch.mmu->mmu_role.base;
 	role.level = level;
@@ -2080,6 +2073,25 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		role.quadrant = quadrant;
 	}
 
+	return role;
+}
+
+static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
+					     gfn_t gfn,
+					     gva_t gaddr,
+					     unsigned level,
+					     int direct,
+					     unsigned int access)
+{
+	bool direct_mmu = vcpu->arch.mmu->direct_map;
+	union kvm_mmu_page_role role;
+	struct hlist_head *sp_list;
+	struct kvm_mmu_page *sp;
+	int collisions = 0;
+	LIST_HEAD(invalid_list);
+
+	role = kvm_mmu_child_role(vcpu, gaddr, level, direct, access);
+
 	sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)];
 	for_each_valid_sp(vcpu->kvm, sp, sp_list) {
 		if (sp->gfn != gfn) {
@@ -2289,15 +2301,22 @@ static int mmu_page_zap_pte(struct kvm *kvm, struct kvm_mmu_page *sp,
 	return 0;
 }
 
-static bool kvm_mmu_child_sp_exists(u64 *sptep, gfn_t gfn)
+static bool kvm_mmu_child_sp_exists(struct kvm_vcpu *vcpu, u64 *sptep,
+				    gfn_t gfn, gva_t gaddr, unsigned level,
+				    int direct, unsigned int access)
 {
+	union kvm_mmu_page_role role;
 	struct kvm_mmu_page *child;
 
 	if (!is_shadow_present_pte(*sptep) || is_large_pte(*sptep))
 		return false;
 
 	child = to_shadow_page(*sptep & PT64_BASE_ADDR_MASK);
-	return child && child->gfn == gfn;
+	if (!child)
+		return false;
+
+	role = kvm_mmu_child_role(vcpu, gaddr, level, direct, access);
+	return child->gfn == gfn && child->role.word == role.word;
 }
 
 static void drop_large_spte(struct kvm_vcpu *vcpu, u64 *sptep)
@@ -3001,7 +3020,8 @@ static int __direct_map(struct kvm_vcpu *vcpu, gpa_t gpa, u32 error_code,
 		if (it.level == level)
 			break;
 
-		if (kvm_mmu_child_sp_exists(it.sptep, base_gfn))
+		if (kvm_mmu_child_sp_exists(vcpu, it.sptep, base_gfn, it.addr,
+					    it.level - 1, true, ACC_ALL))
 			continue;
 
 		drop_large_spte(vcpu, it.sptep);

arch/x86/kvm/mmu/paging_tmpl.h

@@ -661,10 +661,11 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gpa_t addr,
 		clear_sp_write_flooding_count(it.sptep);
 
 		table_gfn = gw->table_gfn[it.level - 2];
+		access = gw->pt_access[it.level - 2];
 		sp = NULL;
-		if (!kvm_mmu_child_sp_exists(it.sptep, table_gfn)) {
+		if (!kvm_mmu_child_sp_exists(vcpu, it.sptep, table_gfn, addr,
+					     it.level - 1, false, access)) {
 			drop_large_spte(vcpu, it.sptep);
-			access = gw->pt_access[it.level - 2];
 			sp = kvm_mmu_get_page(vcpu, table_gfn, addr,
 					      it.level-1, false, access);
 			/*
@@ -721,7 +722,9 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gpa_t addr,
 
 		validate_direct_spte(vcpu, it.sptep, direct_access);
 
-		if (!kvm_mmu_child_sp_exists(it.sptep, base_gfn)) {
+		if (!kvm_mmu_child_sp_exists(vcpu, it.sptep, base_gfn, addr,
+					     it.level - 1, true,
+					     direct_access)) {
 			drop_large_spte(vcpu, it.sptep);
 			sp = kvm_mmu_get_page(vcpu, base_gfn, addr,
 					      it.level - 1, true, direct_access);