Re: [PATCH 5.10.y 11/17] KVM: x86/mmu: Always pass 0 for @quadrant when gptes are 8 bytes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added `WARN_ON_ONCE(role.direct && !role.gpte_is_8_bytes)` is factually incorrect and trivially triggerable by an unprivileged guest, leading to a host-side Denial of Service if `panic_on_warn` is enabled.

Pre-existing issues:
- [High] In direct MMU mappings for non-paging guests, child shadow page allocation incorrectly applies 32-bit quadrant logic and triggers a warning.
--

commit 890f97eba3156db8c16203bd93e23c5a69ffdd15
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: x86/mmu: Always pass 0 for @quadrant when gptes are 8 bytes

The quadrant is only used when gptes are 4 bytes, but
mmu_alloc_{direct,shadow}_roots() pass in a non-zero quadrant for PAE
page directories regardless. Make this less confusing by only passing in
a non-zero quadrant when it is actually necessary.

Message-Id: <20220516232138.1783324-6-dmatlack@google.com>

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index e4759156a2dc8..76d87da1d0712 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3263,9 +3263,10 @@ static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,
>  	struct kvm_mmu_page *sp;
>  
>  	role.level = level;
> +	role.quadrant = quadrant;
>  
> -	if (!role.gpte_is_8_bytes)
> -		role.quadrant = quadrant;
> +	WARN_ON_ONCE(quadrant && role.gpte_is_8_bytes);
> +	WARN_ON_ONCE(role.direct && !role.gpte_is_8_bytes);

[Severity: High]
Can this new warning be triggered by an unprivileged guest?

If a guest runs with TDP disabled (Shadow MMU active) and disables paging
(e.g., boots in real mode), kvm_calc_shadow_mmu_root_page_role() sets
role.base.direct = 1 and role.base.gpte_is_8_bytes = 0.

When mmu_alloc_direct_roots() calls mmu_alloc_root() for a direct MMU
root, won't this unconditionally hit the WARN_ON_ONCE and potentially
cause a host panic if panic_on_warn is enabled?

>  	sp = kvm_mmu_get_page(vcpu, gfn, role);
>  	++sp->root_count;

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but a prior patch in the series
appears to have introduced a similar issue for direct MMU mappings for
non-paging guests.

When a guest operates without paging using the Shadow MMU, the root shadow
page has role.direct = 1 and role.gpte_is_8_bytes = 0.

During page faults, __direct_map() allocates intermediate shadow pages by
calling kvm_mmu_get_child_sp() -> kvm_mmu_child_role():

arch/x86/kvm/mmu/mmu.c:kvm_mmu_child_role() {
        ...
        if (!role.gpte_is_8_bytes) {
                WARN_ON_ONCE(role.level != PG_LEVEL_4K);
                role.quadrant = (sptep - parent_sp->spt) % 2;
        }
        ...
}

Since role.gpte_is_8_bytes == 0, does this erroneously execute the 32-bit
guest PAE-shadowing path, triggering the warning when allocating level 2
shadow pages and incorrectly assigning a quadrant to direct mapping pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626112634.1778506-1-pbonzini@redhat.com?part=11