On Thu, Jul 19, 2018 at 10:31 AM, Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
> Do not expose the address of vmx->nested.current_vmptr to
> kvm_write_guest_virt_system() as the resulting __copy_to_user()
> call will trigger a WARN when CONFIG_HARDENED_USERCOPY is
> enabled.
>
> Opportunistically clean up variable names in handle_vmptrst()
> to improve readability, e.g. vmcs_gva is misleading as the
> memory operand of VMPSTR is plain memory, not a VMCS.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> arch/x86/kvm/vmx.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e30da9a2430c..6688dcf314d3 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8480,21 +8480,20 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
> /* Emulate the VMPTRST instruction */
> static int handle_vmptrst(struct kvm_vcpu *vcpu)
> {
> - unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> - u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> - gva_t vmcs_gva;
> + unsigned long exit_qual = vmcs_readl(EXIT_QUALIFICATION);
> + u32 instr_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> + gpa_t cvp = to_vmx(vcpu)->nested.current_vmptr;
> struct x86_exception e;
> + gva_t gva;
>
> if (!nested_vmx_check_permission(vcpu))
> return 1;
>
> - if (get_vmx_mem_address(vcpu, exit_qualification,
> - vmx_instruction_info, true, &vmcs_gva))
> + if (get_vmx_mem_address(vcpu, exit_qual, instr_info, true, &gva))
> return 1;
> /* *_system ok, nested_vmx_check_permission has verified cpl=0 */
> - if (kvm_write_guest_virt_system(vcpu, vmcs_gva,
> - (void *)&to_vmx(vcpu)->nested.current_vmptr,
> - sizeof(u64), &e)) {
> + if (kvm_write_guest_virt_system(vcpu, gva, (void *)&cvp,
> + sizeof(gpa_t), &e)) {
I actually think sizeof(u64) was better here, since the SDM says:
"64-bit in-memory destination operand <- current-VMCS pointer;" But as
long as a gpa_t is always 64-bits, this is okay.
> kvm_inject_page_fault(vcpu, &e);
> return 1;
> }
> --
> 2.18.0
>
