Re: [Xen-devel] [PATCH v2 6/6] x86/xstate: Fix latent bugs in compress_xsave_states()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.09.16 at 18:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> compress_xsave_states() mustn't read xstate_bv or xcomp_bv before first
> confirming that the input buffer is large enough.  It also doesn't cope with
> compressed input.  Make all of these problems the callers responsbility to
> ensure.
> 
> Simplify the decompression logic by inlining get_xsave_addr().  As xstate_bv
> is previously validated, dest won't ever been NULL.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit, as expressed before I would prefer ...

>  void compress_xsave_states(struct vcpu *v, const void *src, unsigned int 
> size)
>  {
>      struct xsave_struct *xsave = v->arch.xsave_area;
> +    void *dest;
>      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
> -    u64 xstate_bv = ((const struct xsave_struct *)src)->xsave_hdr.xstate_bv;
> -    u64 valid;
> +    u64 xstate_bv, valid;
>  
> -    ASSERT(!xsave_area_compressed(src));
> +    BUG_ON(!v->arch.xcr0_accum);
> +    BUG_ON(size != xstate_ctxt_size(v->arch.xcr0_accum));
> +    BUG_ON(xsave_area_compressed(src));

... if at least the ASSERT() remained one.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel