Re: [Xen-devel] [PATCH 1/5] x86/hvm: Switch hvm_allow_get_param() to use a whitelist

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 05 September 2018 19:12
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Stefano
> Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
> Subject: [PATCH 1/5] x86/hvm: Switch hvm_allow_get_param() to use a
> whitelist
> 
> There are holes in the HVM_PARAM space, some of which are from
> deprecated
> parameters, but toolstack and device models currently have blanket read
> access.
> 
> Rearrange hvm_allow_get_param() to have a whitelist of toolstack-readable
> parameters, with the default case failing with -EINVAL (which subsumes the
> HVM_NR_PARAMS check).
> 
> No expected change for the defined, in-use params.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/x86/hvm/hvm.c | 38 ++++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index c22bf0b..96a6323 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4350,7 +4350,7 @@ static int hvm_allow_get_param(struct domain *d,
> 
>      switch ( a->index )
>      {
> -    /* The following parameters can be read by the guest. */
> +        /* The following parameters can be read by the guest and toolstack. 
> */

Intentional indentation change? I guess so since you do it again below, but 
just checking.

>      case HVM_PARAM_CALLBACK_IRQ:
>      case HVM_PARAM_VM86_TSS:
>      case HVM_PARAM_VM86_TSS_SIZED:
> @@ -4363,18 +4363,39 @@ static int hvm_allow_get_param(struct domain
> *d,
>      case HVM_PARAM_ALTP2M:
>      case HVM_PARAM_X87_FIP_WIDTH:
>          break;
> -    /*
> -     * The following parameters must not be read by the guest
> -     * since the domain may need to be paused.
> -     */
> +
> +        /*
> +         * The following parameters are intended for toolstack usage only.
> +         * Some require the domain to be paused, and therefore may not read
> by
> +         * the domain.
> +         */
> +    case HVM_PARAM_PAE_ENABLED:
>      case HVM_PARAM_IOREQ_PFN:
>      case HVM_PARAM_BUFIOREQ_PFN:
>      case HVM_PARAM_BUFIOREQ_EVTCHN:
> -    /* The remaining parameters should not be read by the guest. */
> -    default:
> +    case HVM_PARAM_VIRIDIAN:
> +    case HVM_PARAM_TIMER_MODE:
> +    case HVM_PARAM_HPET_ENABLED:
> +    case HVM_PARAM_IDENT_PT:
> +    case HVM_PARAM_DM_DOMAIN:
> +    case HVM_PARAM_ACPI_S_STATE:
> +    case HVM_PARAM_VPT_ALIGN:
> +    case HVM_PARAM_NESTEDHVM:
> +    case HVM_PARAM_PAGING_RING_PFN:
> +    case HVM_PARAM_MONITOR_RING_PFN:
> +    case HVM_PARAM_SHARING_RING_PFN:
> +    case HVM_PARAM_TRIPLE_FAULT_REASON:
> +    case HVM_PARAM_IOREQ_SERVER_PFN:
> +    case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> +    case HVM_PARAM_MCA_CAP:
>          if ( d == current->domain )
>              rc = -EPERM;
>          break;
> +
> +        /* Hole, deprecated, or out-of-range. */
> +    default:
> +        rc = -EINVAL;
> +        break;
>      }
> 
>      return rc;
> @@ -4390,9 +4411,6 @@ static int hvmop_get_param(
>      if ( copy_from_guest(&a, arg, 1) )
>          return -EFAULT;
> 
> -    if ( a.index >= HVM_NR_PARAMS )
> -        return -EINVAL;
> -

ASSERT, just in case someone screws up the allow function in future?

>      d = rcu_lock_domain_by_any_id(a.domid);
>      if ( d == NULL )
>          return -ESRCH;
> --
> 2.1.4

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel