Re: [V3 PATCH 6/9] x86/hvm: pkeys, add xstate support for pkeys

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/12/15 09:16, Huaitong Han wrote:
> This patch adds xstate support for pkeys.
>
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
> ---
>  xen/arch/x86/xstate.c        | 7 +++++--
>  xen/include/asm-x86/xstate.h | 4 +++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> index b65da38..db978c4 100644
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -146,12 +146,15 @@ static void __init setup_xstate_comp(void)
>      }
>  }
>  
> -static void *get_xsave_addr(void *xsave, unsigned int xfeature_idx)
> +void *get_xsave_addr(void *xsave, unsigned int xfeature_idx)

This really should take a struct_xsave *xsave, rather than a void pointer.

>  {
>      if ( !((1ul << xfeature_idx) & xfeature_mask) )
>          return NULL;

Now I look at it, this check is bogus.  The check needs to be against
the xsave header in the area, rather than Xen's maximum xfeature_mask. 
A guest might easily have a smaller xcr0 than the maximum Xen is willing
to allow, causing the pointer below to be bogus.

~Andrew