Re: [PATCH v3] KVM: x86: XSAVE/XRSTOR live migration support

[Avi Kivity <avi@redhat.com>]

On 06/11/2010 07:36 AM, Sheng Yang wrote:
> This patch enable save/restore of xsave state.
>
> +static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
> +					struct kvm_xsave *guest_xsave)
> +{
> +	u64 xstate_bv =
> +		*(u64 *)&guest_xsave->region[XSAVE_HDR_OFFSET / sizeof(u32)];
> +	int size;
> +
> +	if (cpu_has_xsave) {
> +		if (xstate_bv&  XSTATE_YMM)
> +			size = XSAVE_YMM_OFFSET + XSAVE_YMM_SIZE;
> +		else
> +			size = XSAVE_HDR_OFFSET + XSAVE_HDR_SIZE;
> +		memcpy(&vcpu->arch.guest_fpu.state->xsave,
> +			guest_xsave->region, size);
>    

This allows userspace to overflow host memory by specifying XSTATE_YMM 
on a host that doesn't support it.

Better to just use the host's size of the structure.

> +	} else {
> +		if (xstate_bv&  ~XSTATE_FPSSE)
> +			return -EINVAL;
> +		size = sizeof(struct i387_fxsave_struct);
> +		memcpy(&vcpu->arch.guest_fpu.state->fxsave,
> +			guest_xsave->region, size);
> +	}
> +	return 0;
> +}
> +
>
> +
> +static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
> +				       struct kvm_xcrs *guest_xcrs)
> +{
> +	int i, r = 0;
> +
> +	if (!cpu_has_xsave)
> +		return -EINVAL;
>    

Too strict?

> +
> +	if (guest_xcrs->nr_xcrs>  KVM_MAX_XCRS)
> +		return -EFAULT;
>    

EFAULT is for faults during access to userspace.  EINVAL or E2BIG.

Need to ensure flags is 0 for forward compatibility.

> +
> +	for (i = 0; i<  guest_xcrs->nr_xcrs; i++)
> +		/* Only support XCR0 currently */
> +		if (guest_xcrs->xcrs[0].xcr == XCR_XFEATURE_ENABLED_MASK) {
> +			r = __kvm_set_xcr(vcpu, XCR_XFEATURE_ENABLED_MASK,
> +				guest_xcrs->xcrs[0].value);
> +			break;
> +		}
> +	if (r)
> +		r = -EFAULT;
>    

EINVAL

> +	return r;
> +}
> +
>    

-- 
error compiling committee.c: too many arguments to function