Messages in this thread |  | | Date | Sun, 22 Jan 2023 10:15:13 +0100 | Subject | Re: [PATCH] KVM: x86/vmx: Do not skip segment attributes if unusable bit is set | From | Paolo Bonzini <> |
| |
On 11/14/22 17:48, Hendrik Borghorst wrote: > When serializing and deserializing kvm_sregs, attributes of the segment > descriptors are stored by user space. For unusable segments, > vmx_segment_access_rights skips all attributes and sets them to 0. > > This means we zero out the DPL (Descriptor Privilege Level) for unusable > entries. > > Unusable segments are - contrary to their name - usable in 64bit mode and > are used by guests to for example create a linear map through the > NULL selector. > > VMENTER checks if SS.DPL is correct depending on the CS segment type. > For types 9 (Execute Only) and 11 (Execute Read), CS.DPL must be equal to > SS.DPL [1]. > > We have seen real world guests setting CS to a usable segment with DPL=3 > and SS to an unusable segment with DPL=3. Once we go through an sregs > get/set cycle, SS.DPL turns to 0. This causes the virtual machine to crash > reproducibly. > > This commit changes the attribute logic to always preserve attributes for > unusable segments. According to [2] SS.DPL is always saved on VM exits, > regardless of the unusable bit so user space applications should have saved > the information on serialization correctly. > > [3] specifies that besides SS.DPL the rest of the attributes of the > descriptors are undefined after VM entry if unusable bit is set. So, there > should be no harm in setting them all to the previous state. > > [1] Intel SDM Vol 3C 26.3.1.2 Checks on Guest Segment Registers > [2] Intel SDM Vol 3C 27.3.2 Saving Segment Registers and Descriptor-Table > Registers > [3] Intel SDM Vol 3C 26.3.2.2 Loading Guest Segment Registers and > Descriptor-Table Registers > > Cc: Alexander Graf <graf@amazon.de> > Signed-off-by: Hendrik Borghorst <hborghor@amazon.de> > --- > arch/x86/kvm/vmx/vmx.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-)
Hi Hendrik,
thanks for the patch! I have queued it now for 6.2-rc6, and added Cc: stable.
Would you mind providing a test in the form of a patch for tools/testing/selftests/kvm? I think a kvm-unit-tests testcase would be harder to do because there's no easy way to force KVM_GET_SREGS/KVM_SET_SREGS.
Thanks,
Paolo
|  |