Re: [PATCH 1/3] x86: Reject bad %dr6/%dr7 values when loading guest state

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.08.2023 15:43, Andrew Cooper wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1074,8 +1074,27 @@ int arch_set_info_guest(
>  #endif
>      flags = c(flags);
>  
> +    if ( !compat )
> +    {
> +        if ( c(debugreg[6]) != (uint32_t)c(debugreg[6]) ||
> +             c(debugreg[7]) != (uint32_t)c(debugreg[7]) )
> +            return -EINVAL;
> +    }
> +
>      if ( is_pv_domain(d) )
>      {
> +        /*
> +         * Prior to Xen 4.11, dr5 was used to hold the emulated-only
> +         * subset of dr7, and dr4 was unused.
> +         *
> +         * In Xen 4.11 and later, dr4/5 are written as zero, ignored for
> +         * backwards compatibility, and dr7 emulation is handled
> +         * internally.
> +         */
> +        for ( i = 0; i < ARRAY_SIZE(v->arch.dr); i++ )
> +            if ( !access_ok(c(debugreg[i]), sizeof(long)) )
> +                return -EINVAL;
> +
>          if ( !compat )
>          {
>              if ( !is_canonical_address(c.nat->user_regs.rip) ||

One more thing here: v->arch.dr is an array of 4 elements, i.e. doesn't
cover %dr4 and up. That's not directly visible here, though, so the
comment ahead of the loop talking about those other 4 registers is a
little misleading. Would you mind moving it below the loop?

Jan