Re: [PATCH v2 1/5] xen/domain: Introduce a new sanitise_domain_config() helper

From: Julien Grall <julien.grall@arm.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>, Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH v2 1/5] xen/domain: Introduce a new sanitise_domain_config() helper
Date: Wed, 14 Nov 2018 18:49:40 +0000	[thread overview]
Message-ID: <2edcb84c-edb3-4351-2f22-6c1aad8f41af@arm.com> (raw)
In-Reply-To: <1542039391-32406-2-git-send-email-andrew.cooper3@citrix.com>

Hi Andrew,

On 12/11/2018 16:16, Andrew Cooper wrote:
> Call it from the head of domain_create() (before doing any memory
> allocations), which will apply the checks to dom0 as well as domU's.
> 
> For now, just subsume the XEN_DOMCTL_CDF_* check from XEN_DOMCTL_createdomain.
> 
> In an effort to aid future developoment, leave a debug printk() identifying
> the cause of sanitisation failures.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
> v2:
>   * Rename to sanitise_domain_config()
>   * Leave a dprintk() behind
> ---
>   xen/common/domain.c | 18 ++++++++++++++++++
>   xen/common/domctl.c |  9 ---------
>   2 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index d6650f0..22aa634 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -288,6 +288,21 @@ static void _domain_destroy(struct domain *d)
>       free_domain_struct(d);
>   }
>   
> +static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> +{
> +    if ( config->flags & ~(XEN_DOMCTL_CDF_hvm_guest |
> +                           XEN_DOMCTL_CDF_hap |
> +                           XEN_DOMCTL_CDF_s3_integrity |
> +                           XEN_DOMCTL_CDF_oos_off |
> +                           XEN_DOMCTL_CDF_xs_domain) )
> +    {
> +        dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
>   struct domain *domain_create(domid_t domid,
>                                struct xen_domctl_createdomain *config,
>                                bool is_priv)
> @@ -297,6 +312,9 @@ struct domain *domain_create(domid_t domid,
>              INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 };
>       int err, init_status = 0;
>   
> +    if ( config && (err = sanitise_domain_config(config)) )
> +        return ERR_PTR(err);
> +
>       if ( (d = alloc_domain_struct()) == NULL )
>           return ERR_PTR(-ENOMEM);
>   
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index b294881..d08b627 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -498,15 +498,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>           domid_t        dom;
>           static domid_t rover = 0;
>   
> -        ret = -EINVAL;
> -        if ( (op->u.createdomain.flags &
> -             ~(XEN_DOMCTL_CDF_hvm_guest
> -               | XEN_DOMCTL_CDF_hap
> -               | XEN_DOMCTL_CDF_s3_integrity
> -               | XEN_DOMCTL_CDF_oos_off
> -               | XEN_DOMCTL_CDF_xs_domain)) )
> -            break;
> -
>           dom = op->domain;
>           if ( (dom > 0) && (dom < DOMID_FIRST_RESERVED) )
>           {
> 

-- 
Julien Grall

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