linux-kernel - Re: [PATCH 2/2] KVM: SVM: Raise #UD if VMMCALL instruction is not intercepted

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <qsigylx7qbq6ymbouw5rfuseqwb6s6fczhbusxqe5e46bscxxz@2cmi2okx7cte>
Date: Wed, 7 Jan 2026 00:02:40 +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 2/2] KVM: SVM: Raise #UD if VMMCALL instruction is not
 intercepted

On Tue, Jan 06, 2026 at 03:38:57PM -0800, Sean Christopherson wrote:
> On Tue, Jan 06, 2026, Yosry Ahmed wrote:
> > On Tue, Jan 06, 2026 at 10:29:59AM -0800, Sean Christopherson wrote:
> > > > +static int vmmcall_interception(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +	/*
> > > > +	 * If VMMCALL from L2 is not intercepted by L1, the instruction raises a
> > > > +	 * #UD exception
> > > > +	 */
> > > 
> > > Mentioning L2 and L1 is confusing.  It reads like arbitrary KVM behavior.  And
> > > IMO the most notable thing is what's missing: an intercept check.  _That_ is
> > > worth commenting, e.g.
> > > 
> > > 	/*
> > > 	 * VMMCALL #UDs if it's not intercepted, and KVM reaches this point if
> > > 	 * and only if the VMCALL intercept is not set in vmcb12.
> > 
> > Nit: VMMCALL
> > 
> > > 	 */
> > > 
> > 
> > Would it be too paranoid to WARN if the L1 intercept is set here?
> 
> Yes.  At some point we have to rely on not being completely inept :-D, and more
> importantly this is something that should be trivial easy to validate via tests.
> 
> My hesitation for such a check is that adding a WARN here begs the question of
> what makes _this_ particular handler special, i.e. why doesn't every other handler
> also check that an exit shouldn't have been routed to L1?  At that point we'd be
> replicating much of the routing logic into every exit handler.

I briefly thought about this, and I thought since we're explicitly
calling out the dependency on L1 not intercepting this here, a WARN
would make sense. Anyway, your argument make sense. I was going to
suggested adding a WARN in svm_invoke_exit_handler() instead, something
like:

WARN_ON_ONCE(vmcb12_is_intercept(&svm->nested.ctl, exit_code));

But this doesn't work for all cases (e.g. SVM_EXIT_MSR, SVM_EXIT_IOIO,
etc). We can exclude these cases, but this point maintaining the WARN
becomes a burden in itself. I give up :)

> 
> And it _still_ wouldn't guarantee correctness, e.g. wouldn't detect the case where
> KVM incorrectly forwarded a VMMCALL to L1, i.e. we still need the aforementioned
> tests, and so I see the WARN as an overall net-negative.
> 
> > WARN_ON_ONCE(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_VMMCALL));