Messages in this thread | | | Subject | Re: [PATCH 09/10] KVM: nVMX: include shadow vmcs12 in nested state | From | Paolo Bonzini <> | Date | Tue, 31 Jul 2018 09:39:42 +0200 |
| |
On 30/07/2018 21:11, Jim Mattson wrote: > Does this work with CONFIG_HARDENED_USERCOPY?
Yes, kmalloc objects are exempt from the check (see new_kmalloc_cache).
> Is VMCS12_SIZE better than sizeof(*vmcs12)? What if we are migrating > to a destination where sizeof(*vmcs12) is larger than it is on the > source?
Either those fields won't be used by the destination due to the VMX MSR values, or migration should have failed due to invalid VMX MSR values.
VMCS12_SIZE is consistent with handle_vmptrld and nested_release_vmcs12. If we use sizeof(*vmcs12) in vmx_get_nested_state, we will have to adjust the copy from/to cached_vmcs12 and cached_shadow_vmcs12 to also use sizeof(*vmcs12), otherwise:
1) nested_release_vmcs12 can leak arbitrary kernel data to userspace, because the cached_vmcs12 and cached_shadow_vmcs12 are not zeroed when allocated.
2) even if we zeroed the allocation in enter_vmx_operation, the guest could observe a migration because it would change the padding at the end of the VMCS changes to zeroes; this is strictly speaking not an issue, but it's ugly.
Paolo
> If userspace doesn't zero the buffer before calling the ioctl, > then the destination may interpret nonsense as actual VMCS field > values. However, if we copy the entire page from the kernel, then we > know that anything beyond the source's sizeof(*vmcs12) will be zero.
| |