Re: [PATCH 2/4] KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new bounds check falsely triggers KVM_BUG_ON for valid PAE page walks on 32-bit KVM hosts.
--

commit c8c785606f8b5ac435db8c42d576eaf8f88c2fcd
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

This commit extends the paging levels check to ensure KVM doesn't attempt to
walk more levels than the MMU template provides, preventing out-of-bounds
array accesses.

> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 27427e7f22fa0..46a0f7796e55a 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -368,13 +368,14 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>  	pte_access = ~0;
>  
>  	/*
> -	 * Queue a page fault for injection if this assertion fails, as callers
> -	 * assume that walker.fault contains sane info on a walk failure.  I.e.
> -	 * avoid making the situation worse by inducing even worse badness
> -	 * between when the assertion fails and when KVM kicks the vCPU out to
> -	 * userspace (because the VM is bugged).
> +	 * Queue a page fault for injection if any of the below assertions fail,
> +	 * as callers assume that walker.fault contains sane info on a walk
> +	 * failure.  I.e. avoid making the situation worse by inducing even
> +	 * worse badness between when the assertion fails and when KVM kicks
> +	 * the vCPU out to userspace (because the VM is bugged).
>  	 */
> -	if (KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm))
> +	if (KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm) ||
> +	    KVM_BUG_ON(w->cpu_role.base.level > PT_MAX_FULL_LEVELS, vcpu->kvm))

[Severity: High]
Does this new check falsely trigger KVM_BUG_ON for valid PAE page walks on
32-bit KVM hosts?

When a PAE guest runs on a 32-bit KVM host (!CONFIG_X86_64), KVM assigns
w->cpu_role.base.level = PT32E_ROOT_LEVEL (3). During a page walk,
walk_addr_generic() is compiled for PTTYPE == 64. In this context,
PT_MAX_FULL_LEVELS is defined as 2.

The check w->cpu_role.base.level > PT_MAX_FULL_LEVELS evaluates to 3 > 2,
which unconditionally bugs the VM.

The array accesses in the subsequent loop are safe because walker->level is
explicitly decremented from 3 to 2 in the PAE-specific block earlier in the
function, ensuring the max_level used for array indexing does not exceed 2.

Should this check use the adjusted walker->max_level instead of the original
unmodified w->cpu_role.base.level?

>  		goto error;
>  
>  	++walker->level;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826211844.884951-1-seanjc@google.com?part=2