On Tue, Mar 12, 2024 at 09:07:11AM -0700, Sean Christopherson wrote: > On Tue, Mar 12, 2024, Kevin Tian wrote: > > > From: Sean Christopherson <seanjc@xxxxxxxxxx> > > > 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@xxxxxxxxx > > > 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. > What about add a new module parameter to turn on honoring guest for non-coherent DMAs on CPUs without self-snoop? A previous example is VFIO's "allow_unsafe_interrupts" parameter. > 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. In this case, memory aliasing may only lead to guest not function well, since guest is not using WC/UC (which can bypass host initialization data in cache). But if guest has any chance to read information not intended to it, I believe we need to fix it as well. > > > 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. What if the guest is a totally malicious one? Previously we trust the guest in the case of noncoherent DMA is because we believe a malicious guest will only meet data corruption and shoot his own foot. But as Jason raised the security problem in another mail thread [1], this will expose security hole if CPUs have no self-snoop. So, we need to fix it, right? + Jason, in case I didn't understand this problem correctly. [1] https://lore.kernel.org/all/20240108153818.GK50406@xxxxxxxxxx/ > > 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.
