Re: [PATCH 5.15.y 4/8] 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 1 potential issue(s) to consider:
- [High] The newly added WARN_ON_ONCE(role.direct && !role.gpte_is_8_bytes) in mmu_alloc_root incorrectly triggers when KVM uses the shadow MMU for a guest with paging disabled (CR0.PG=0), enabling a guest-triggerable host Denial of Service.
--

commit f4ad49b2fb4e69e5a09cb3c4ffc7f0aaaa670e2f
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 3a5ed96703776..dbc18d4cc572c 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3444,9 +3444,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]
Could this warning in mmu_alloc_root() inadvertently trigger a
guest-triggerable host denial of service?

If KVM uses the shadow MMU (for example, ept=0 or nested without TDP) and
the guest disables paging (CR0.PG=0), KVM falls back to direct mapping mode.

In this state, kvm_calc_shadow_mmu_root_page_role() in arch/x86/kvm/mmu/mmu.c
sets role.base.direct = true, but kvm_calc_shadow_root_page_role_common()
leaves role.base.gpte_is_8_bytes as false since paging is disabled.

When handling the next memory access, wouldn't the call chain
kvm_mmu_load() -> mmu_alloc_direct_roots() -> mmu_alloc_root()
cause this warning to unconditionally evaluate to true and trigger a kernel
panic if the host has panic_on_warn enabled?

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

[ ... ]

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