Re: [PATCH 1/2] KVM: x86: Fix partially uninitialized integer in emulate_pop

[Julian Stecklina]

On Fri, 2023-10-06 at 00:56 +0000, Sean Christopherson wrote:
> On Thu, Oct 05, 2023, Julian Stecklina wrote:
> > On Wed, 2023-10-04 at 08:07 -0700, Sean Christopherson wrote:
> > > 
> > > NAK, this will break em_leave() as it will zero RBP regardless of how many
> > > bytes
> > > are actually supposed to be written.  Specifically, KVM would incorrectly
> > > clobber
> > > RBP[31:16] if LEAVE is executed with a 16-bit stack.
> > 
> > Thanks, Sean! Great catch. I didn't see this. Is there already a test suite
> > for
> > this?
> 
> No, I'm just excessively paranoid when it comes to the emulator :-)

I'll look into whether some testing can be added to kvm-unit-tests or maybe some
other test harness.

> It pains me a bit to say this, but I think we're best off leaving the emulator
> as-is, and relying on things like fancy compiler features, UBSAN, and fuzzers
> to
> detect any lurking bugs.

I'm have a fuzzing setup for the emulator in userspace. This issue was detected
by MSAN. :) I'll make this available when it's in a better shape.

So if you don't strongly mind , I would still initialize the places where the
fuzzer can show that the code hands uninialized data around. At the least, it
will make other fuzzing efforts a bit easier. But I do understand that changes
need to be conservative.

Btw, what are the cases where ret far, iret etc (basically anything you wouldn't
expect for MMIO) are handled by the KVM emulator without the guest doing
anything fishy? 

Julian