On 2/27/2020 8:11 AM, Sean Christopherson wrote:
On Tue, Feb 11, 2020 at 02:34:18PM +0100, Paolo Bonzini wrote:
On 11/02/20 14:22, Thomas Gleixner wrote:
Paolo Bonzini <pbonzini@xxxxxxxxxx> writes:
On 03/02/20 16:16, Xiaoyao Li wrote:
A sane guest should never tigger emulation on a split-lock access, but
it cannot prevent malicous guest from doing this. So just emulating the
access as a write if it's a split-lock access to avoid malicous guest
polluting the kernel log.
Saying that anything doing a split lock access is malicious makes little
sense.
Correct, but we also have to accept, that split lock access can be used
in a malicious way, aka. DoS.
Indeed, a more accurate emulation such as temporarily disabling
split-lock detection in the emulator would allow the guest to use split
lock access as a vehicle for DoS, but that's not what the commit message
says. If it were only about polluting the kernel log, there's
printk_ratelimited for that. (In fact, if we went for incorrect
emulation as in this patch, a rate-limited pr_warn would be a good idea).
It is much more convincing to say that since this is pretty much a
theoretical case, we can assume that it is only done with the purpose of
DoS-ing the host or something like that, and therefore we kill the guest.
The problem with "kill the guest", and the reason I'd prefer to emulate the
split-lock as a write, is that killing the guest in this case is annoyingly
difficult.
Returning X86EMUL_UNHANDLEABLE / EMULATION_FAILED gets KVM to
handle_emulation_failure(), but handle_emulation_failure() will only "kill"
the guest if emulation failed in L1 CPL==0. For all other modes, it will
inject a #UD and resume the guest. KVM also injects a #UD for L1 CPL==0,
but that's the least annoying thing.
Adding a new emulation type isn't an option because this code can be
triggered through normal emulation. A new return type could be added for
split-lock, but that's code I'd really not add, both from an Intel
perspective and a KVM maintenance perspective. And, we'd still have the
conundrum of what to do if/when split-lock #AC is exposed to L1, e.g. in
that case, KVM should inject an #AC into L1, not kill the guest. Again,
totally doable, but ugly and IMO an unnecessary maintenance burden.
I completely agree that poorly emulating the instruction from the (likely)
malicious guest is a hack, but it's a simple and easy to maintain hack.
Paolo,
What's your opinion about above?
Split lock detection is essentially a debugging feature, there's a
reason why the MSR is called "TEST_CTL". So you don't want to make the
The fact that it ended up in MSR_TEST_CTL does not say anything. That's
where they it ended up to be as it was hastily cobbled together for
whatever reason.
Or perhaps it was there all the time in test silicon or something like
that... That would be a very plausible reason for all the quirks behind it.
Paolo
