On Wed, 2023-10-04 at 08:07 -0700, Sean Christopherson wrote: > On Wed, Oct 04, 2023, Julian Stecklina wrote: > > Most code gives a pointer to an uninitialized unsigned long as dest in > > emulate_pop. len is usually the word width of the guest. > > > > If the guest runs in 16-bit or 32-bit modes, len will not cover the > > whole unsigned long and we end up with uninitialized data in dest. > > > > Looking through the callers of this function, the issue seems > > harmless, but given that none of this is performance critical, there > > should be no issue with just always initializing the whole value. > > > > Fix this by explicitly requiring a unsigned long pointer and > > initializing it with zero in all cases. > > 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? > I generally like defense-in-depth approaches, but zeroing data that the caller > did not ask to be written is not a net positive. I'll rewrite the patch to just initialize variables where they are currently not. This should be a bit more conservative and have less risk of breaking anything. Julian
