linux-kernel - Re: [PATCH 1/3] KVM: SVM: Fix nested NPF injection to set PFERR_GUEST_{PAGE,FINAL}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <sdyb3l4ihmcd7uxb6wivkyknmzy4bcctqyyidxq7hr2d2jfs6e@iz3fhfp6t4ss>
Date: Thu, 22 Jan 2026 00:45:23 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Kevin Cheng <chengkev@...gle.com>, pbonzini@...hat.com, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] KVM: SVM: Fix nested NPF injection to set
 PFERR_GUEST_{PAGE,FINAL}_MASK

On Wed, Jan 21, 2026 at 02:07:56PM -0800, Sean Christopherson wrote:
> On Wed, Jan 21, 2026, Kevin Cheng wrote:
> > When KVM emulates an instruction for L2 and encounters a nested page
> > fault (e.g., during string I/O emulation), nested_svm_inject_npf_exit()
> > injects an NPF to L1. However, the code incorrectly hardcodes
> > (1ULL << 32) for exit_info_1's upper bits when the original exit was
> > not an NPF. This always sets PFERR_GUEST_FINAL_MASK even when the fault
> > occurred on a page table page, preventing L1 from correctly identifying
> > the cause of the fault.
> > 
> > Set PFERR_GUEST_PAGE_MASK in the error code when a nested page fault
> > occurs during a guest page table walk, and PFERR_GUEST_FINAL_MASK when
> > the fault occurs on the final GPA-to-HPA translation.
> > 
> > Widen error_code in struct x86_exception from u16 to u64 to accommodate
> > the PFERR_GUEST_* bits (bits 32 and 33).
> 
> Please do this in a separate patch.  Intel CPUs straight up don't support 32-bit
> error codes, let alone 64-bit error codes, so this seemingly innocuous change
> needs to be accompanied by a lengthy changelog that effectively audits all usage
> to "prove" this change is ok.

Semi-jokingly, we can add error_code_hi to track the high bits and
side-step the problem for Intel (dejavu?).

> 
> > Update nested_svm_inject_npf_exit() to use fault->error_code directly
> > instead of hardcoding the upper bits. Also add a WARN_ON_ONCE if neither
> > PFERR_GUEST_FINAL_MASK nor PFERR_GUEST_PAGE_MASK is set, as this would
> > indicate a bug in the page fault handling code.
> > 
> > Signed-off-by: Kevin Cheng <chengkev@...gle.com>
[..]
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index de90b104a0dd5..f8dfd5c333023 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -40,18 +40,17 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
> >  	struct vmcb *vmcb = svm->vmcb;
> >  
> >  	if (vmcb->control.exit_code != SVM_EXIT_NPF) {
> > -		/*
> > -		 * TODO: track the cause of the nested page fault, and
> > -		 * correctly fill in the high bits of exit_info_1.
> > -		 */
> > -		vmcb->control.exit_code = SVM_EXIT_NPF;
> > -		vmcb->control.exit_info_1 = (1ULL << 32);
> > +		vmcb->control.exit_info_1 = fault->error_code;
> >  		vmcb->control.exit_info_2 = fault->address;
> >  	}
> >  
> > +	vmcb->control.exit_code = SVM_EXIT_NPF;
> >  	vmcb->control.exit_info_1 &= ~0xffffffffULL;
> >  	vmcb->control.exit_info_1 |= fault->error_code;
> 
> So... what happens when exit_info_1 already has PFERR_GUEST_PAGE_MASK, and then
> @fault sets PFERR_GUEST_FINAL_MASK?  Presumably that can't/shouldn't happen,
> but nothing in the changelog explains why such a scenario is
> impossible, and nothing in the code hardens KVM against such goofs.

I guess we can update the WARN below to check for that as well, and
fallback to the current behavior (set PFERR_GUEST_FINAL_MASK):

	fault_stage = vmcb->control.exit_info_1 &
			(PFERR_GUEST_FINAL_MASK | PFERR_GUEST_PAGE_MASK);
	if (WARN_ON_ONCE(fault_stage != PFERR_GUEST_FINAL_MASK &&
			 fault_stage != PFERR_GUEST_PAGE_MASK))
		vmcb->control.exit_info_1 |= PFERR_GUEST_FINAL_MASK;

> 
> > +	WARN_ON_ONCE(!(vmcb->control.exit_info_1 &
> > +		       (PFERR_GUEST_FINAL_MASK | PFERR_GUEST_PAGE_MASK)));
> > +
> >  	nested_svm_vmexit(svm);
> >  }
> >  
> > -- 
> > 2.52.0.457.g6b5491de43-goog
> >