linux-kernel - Re: [PATCH 3/3] KVM: nSVM: Avoid incorrect injection of SVM_EXIT_CR0_SEL

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: <ygndurqaaapgrr2omlkdqa24sn6dubndp5cmsgasnobdwhcdql@gfysa3jtfjha>
Date: Wed, 5 Nov 2025 21:35:38 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Jim Mattson <jmattson@...gle.com>, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel
Subject: Re: [PATCH 3/3] KVM: nSVM: Avoid incorrect injection of
 SVM_EXIT_CR0_SEL_WRITE

On Wed, Nov 05, 2025 at 12:37:37PM -0800, Sean Christopherson wrote:
> On Wed, Nov 05, 2025, Yosry Ahmed wrote:
> > On Wed, Nov 05, 2025 at 11:48:27AM -0800, Sean Christopherson wrote:
> > > On Fri, Oct 24, 2025, Yosry Ahmed wrote:
> > Looks good with a minor nit:
> > 
> > > 
> > > 		/*
> > > 		 * Adjust the exit code accordingly if a CR other than CR0 is
> > > 		 * being written, and skip straight to the common handling as
> > > 		 * only CR0 has an additional selective intercept.
> > > 		 */
> > > 		if (info->intercept == x86_intercept_cr_write && info->modrm_reg) {
> > > 			icpt_info.exit_code += info->modrm_reg;
> > > 			break;
> > > 		}
> > > 
> > > 		/*
> > > 		 * Convert the exit_code to SVM_EXIT_CR0_SEL_WRITE if L1 set
> > > 		 * INTERCEPT_SELECTIVE_CR0 but not INTERCEPT_CR0_WRITE, as the
> > > 		 * unconditional intercept has higher priority.
> > > 		 */
> > 
> > We only convert the exict_code to SVM_EXIT_CR0_SEL_WRITE if other
> > conditions are true below. So maybe "Check if the exit_code needs to be
> > converted to.."?
> 
> Ouch, good point.  I keep forgetting that the common code below this needs to
> check the exit_code against the intercept enables.  How about this?

Looks good.

> 
> 		/*
> 		 * Convert the exit_code to SVM_EXIT_CR0_SEL_WRITE if a
> 		 * selective CR0 intercept is triggered (the common logic will
> 		 * treat the selective intercept as being enabled).  Note, the
> 		 * unconditional intercept has higher priority, i.e. this is
> 		 * only relevant if *only* the selective intercept is enabled.
> 		 */
> 
> > 
> > > 		if (vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_CR0_WRITE) ||
> > > 		    !(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_SELECTIVE_CR0)))
> > > 			break;
> > > 
> > > 
> > > > -		    info->intercept == x86_intercept_clts)
> > > > +		    vmcb12_is_intercept(&svm->nested.ctl,
> > > > +					INTERCEPT_CR0_WRITE) ||
> > > > +		    !(vmcb12_is_intercept(&svm->nested.ctl,
> > > > +					  INTERCEPT_SELECTIVE_CR0)))
> > > 
> > > Let these poke out.
> > 
> > Sure. Do you prefer a new version with this + your fixup above, or will
> > you fix them up while applying?
> 
> If you're happy with it, I'll just fixup when applying.

More than happy :)