On Mon, Jun 14, 2010 at 12:15:10PM +0300, Avi Kivity wrote:
> On 06/13/2010 03:28 PM, Nadav Har'El wrote:
> >This patch implements the VMPTRST instruction.
> >
> >Signed-off-by: Nadav Har'El<nyh@xxxxxxxxxx>
> >---
> >--- .before/arch/x86/kvm/x86.c 2010-06-13 15:01:29.000000000 +0300
> >+++ .after/arch/x86/kvm/x86.c 2010-06-13 15:01:29.000000000 +0300
> >@@ -3301,7 +3301,7 @@ static int kvm_read_guest_virt_system(gv
> > return kvm_read_guest_virt_helper(addr, val, bytes, vcpu, 0, error);
> > }
> >
> >-static int kvm_write_guest_virt_system(gva_t addr, void *val,
> >+int kvm_write_guest_virt_system(gva_t addr, void *val,
> > unsigned int bytes,
> > struct kvm_vcpu *vcpu,
> > u32 *error)
>
> write_guest_virt_system() is used by writes which need to ignore the
> cpl, for example when a cpl 3 instruction loads a segment, the
> processor needs to update the accessed flag even though it is only
> accessible to cpl 0. That's not your case, you need the ordinary
> write_guest_virt().
>
> Um, I see there is no kvm_write_guest_virt(), you'll have to introduce it.
>
the code uses this function after checking cpl to be zero, so may be it
is ok, not to pretty though. I was actually hoping to get rid of all
kvm_(read|write)_guest_virt* and replace existing uses with
emulator_(read|write)_emulated, but this patch series adds more users
that will be hard to replace :(
> >
> >+/* Emulate the VMPTRST instruction */
> >+static int handle_vmptrst(struct kvm_vcpu *vcpu)
> >+{
> >+ int r = 0;
> >+ unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> >+ u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> >+ gva_t vmcs_gva;
> >+
> >+ if (!nested_vmx_check_permission(vcpu))
> >+ return 1;
> >+
> >+ vmcs_gva = get_vmx_mem_address(vcpu, exit_qualification,
> >+ vmx_instruction_info);
> >+ if (vmcs_gva == 0)
> >+ return 1;
>
> What's wrong with gva 0? It's favoured by exploiters everywhere.
>
> >+ r = kvm_write_guest_virt_system(vmcs_gva,
> >+ (void *)&to_vmx(vcpu)->nested.current_vmptr,
> >+ sizeof(u64), vcpu, NULL);
> >+ if (r) {
>
> Check against the X86EMUL return codes. You'll need to inject a
> page fault on failure.
>
> >+ printk(KERN_INFO "%s failed to write vmptr\n", __func__);
> >+ return 1;
> >+ }
> >+ clear_rflags_cf_zf(vcpu);
> >+ skip_emulated_instruction(vcpu);
> >+ return 1;
> >+}
> >+
>
> --
> error compiling committee.c: too many arguments to function
>
> --
> 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
--
Gleb.
--
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
