Message-ID: <ZfB9rzqOWmbaOeHd@google.com>
Date: Tue, 12 Mar 2024 09:07:11 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Kevin Tian <kevin.tian@...el.com>
Cc: Yan Y Zhao <yan.y.zhao@...el.com>, Paolo Bonzini <pbonzini@...hat.com>, 
	Lai Jiangshan <jiangshanlai@...il.com>, "Paul E. McKenney" <paulmck@...nel.org>, 
	Josh Triplett <josh@...htriplett.org>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>, 
	"rcu@...r.kernel.org" <rcu@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Yiwei Zhang <zzyiwei@...gle.com>
Subject: Re: [PATCH 5/5] KVM: VMX: Always honor guest PAT on CPUs that support self-snoop

On Tue, Mar 12, 2024, Kevin Tian wrote:
> > From: Sean Christopherson <seanjc@...gle.com>
> > Sent: Tuesday, March 12, 2024 8:26 AM
> > 
> > On Mon, Mar 11, 2024, Yan Zhao wrote:
> > > For the case of !static_cpu_has(X86_FEATURE_SELFSNOOP) &&
> > > kvm_arch_has_noncoherent_dma(vcpu->kvm), I think we at least should warn
> > > about unsafe before honoring guest memory type.
> > 
> > I don't think it gains us enough to offset the potential pain such a
> > message would bring.  Assuming the warning isn't outright ignored, the most
> > likely scenario is that the warning will cause random end users to worry
> > that the setup they've been running for years is broken, when in reality
> > it's probably just fine for their
> > use case.
> 
> Isn't the 'worry' necessary to allow end users evaluate whether "it's
> probably just fine for their use case"?

Realistically, outside of large scale deployments, no end user is going to be able
to make that evaluation, because practically speaking it requires someone with
quite low-level hardware knowledge to be able to make that judgment call.  And
counting by number of human end users (as opposed to number of VMs being run), I
am willing to bet that the overwhelming majority of KVM users aren't kernel or
systems engineers.

Understandably, users tend to be alarmed by (or suspicious of) new warnings that
show up.  E.g. see the ancient KVM_SET_TSS_ADDR pr_warn[*].  And recently, we had
an internal bug report filed against KVM because they observed a performance
regression when booting a KVM guest, and saw a new message about some CPU
vulnerability being mitigated on VM-Exit that showed up in their *guest* kernel.

In short, my concern is that adding a new pr_warn() will generate noise for end
users *and* for KVM developers/maintainers, because even if we phrase the message
to talk specifically about "untrusted workloads", the majority of affected users
will not have the necessary knowledge to make an informed decision.

[*] https://lore.kernel.org/all/f1afa6c0-cde2-ab8b-ea71-bfa62a45b956@tarentde

> I saw the old comment already mentioned that doing so may lead to unexpected
> behaviors. But I'm not sure whether such code-level caveat has been visible
> enough to end users.

Another point to consider: KVM is _always_ potentially broken on such CPUs, as
KVM forces WB for guest accesses.  I.e. KVM will create memory aliasing if the
host has guest memory mapped as non-WB in the PAT, without non-coherent DMA
exposed to the guest.

> > I would be quite surprised if there are people running untrusted workloads
> > on 10+ year old silicon *and* have passthrough devices and non-coherent
> > IOMMUs/DMA.
> 
> this is probably true.
> 
> > And anyone exposing a device directly to an untrusted workload really
> > should have done their homework.
> 
> or they run trusted workloads which might be tampered by virus to
> exceed the scope of their homework. 😊

If a workload is being run in a KVM guest for host isolation/security purposes,
and a device was exposed to said workload, then I would firmly consider analyzing
the impact of a compromised guest to be part of their homework.

> > And it's not like we're going to change KVM's historical behavior at this point.
> 
> I agree with your point of not breaking userspace. But still think a warning
> might be informative to let users evaluate their setup against a newly
> identified "unexpected behavior"  which has security implication beyond
> the guest, while the previous interpretation of "unexpected behavior" 
> might be that the guest can at most shoot its own foot...

If this issue weren't limited to 10+ year old hardware, I would be more inclined
to add a message.  But at this point, realistically the only thing KVM would be
saying is "you're running old hardware, that might be unsafe in today's world".

For users that care about security, we'd be telling them something they already
know (and if they don't know, they've got bigger problems).  And for everyone
else, it'd be scary noise without any meaningful benefit.

Powered by blists - more mailing lists