[Xen-devel] Re: [Patch 4/4] Refining Xsave/Xrestore support - Version 2

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 29.10.10 at 03:49, Haitao Shan <maillists.shan@xxxxxxxxx> wrote:
>+            if ( evc->size != PV_XSAVE_SIZE ||
>+                 evc->xfeature_mask != xfeature_mask )
>+            {
>+                ret = EFAULT;

This ought to be negative, and perhaps another error code would be
better to pick here.

>+                goto vcpuextstate_out;
>+            }
>...
>+            if ( evc->size > PV_XSAVE_SIZE )
>+                goto vcpuextstate_out;

What if evc->size < PV_XSAVE_SIZE? You're still copying
xsave_cntxt_size bytes in the final copy_from_guest_offset().

Also, you're copying directly from the user buffer into struct vcpu
fields, so you'll leave inconsistent state there if the second or
third copy fails but at least the first succeeded. I think you need
to copy the full input structure first, check for validity, and only
then put the fields into the respective struct vcpu ones.

>+#define CPU_XSAVE_CODE  16

I forgot to ask already on your first submission why this isn't
using DECLARE_HVM_SAVE_TYPE().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel