On Sun, May 08, 2011 at 11:21:22AM +0300, Nadav Har'El wrote:
> This patch implements the VMPTRLD instruction.
>
> Signed-off-by: Nadav Har'El <nyh@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx.c | 62 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> --- .before/arch/x86/kvm/vmx.c 2011-05-08 10:43:19.000000000 +0300
> +++ .after/arch/x86/kvm/vmx.c 2011-05-08 10:43:19.000000000 +0300
> @@ -4814,6 +4814,66 @@ static int handle_vmclear(struct kvm_vcp
> return 1;
> }
>
> +/* Emulate the VMPTRLD instruction */
> +static int handle_vmptrld(struct kvm_vcpu *vcpu)
> +{
> + struct vcpu_vmx *vmx = to_vmx(vcpu);
> + gva_t gva;
> + gpa_t vmcs12_addr;
> + struct x86_exception e;
> +
> + if (!nested_vmx_check_permission(vcpu))
> + return 1;
> +
> + if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
> + vmcs_read32(VMX_INSTRUCTION_INFO), &gva))
> + return 1;
> +
> + if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &vmcs12_addr,
> + sizeof(vmcs12_addr), &e)) {
> + kvm_inject_page_fault(vcpu, &e);
> + return 1;
> + }
> +
> + if (!IS_ALIGNED(vmcs12_addr, PAGE_SIZE)) {
> + nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_INVALID_ADDRESS);
> + skip_emulated_instruction(vcpu);
> + return 1;
> + }
> +
> + if (vmx->nested.current_vmptr != vmcs12_addr) {
> + struct vmcs12 *new_vmcs12;
> + struct page *page;
> + page = nested_get_page(vcpu, vmcs12_addr);
> + if (page == NULL) {
> + nested_vmx_failInvalid(vcpu);
This can access a NULL current_vmcs12 pointer, no? Apparently other
code paths are vulnerable to the same issue (as in allowed to execute
before vmtprld maps guest VMCS). Perhaps a BUG_ON on get_vmcs12 could be
helpful.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
