|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks
On 05/05/15 11:25, Paul Durrant wrote:
> Some parameters can only (validly) be set once. Some should not be set
> by a guest for its own domain, and others must not be set since they
> require the domain to be paused. Consolidate these checks, along with
> the XSM check, in a new hvm_allow_set_param() function for clarity.
>
> Also, introduce hvm_allow_get_param() for similar reasons.
>
> NOTE: This patch leaves the guest accessible parameter checks as
> black-lists and does not make any changes to which parameters
> can be accessed. A subsequent patch will tighten up these
> checks by changing from black-lists to white-lists.
>
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> xen/arch/x86/hvm/hvm.c | 153
> ++++++++++++++++++++++++++++--------------------
> 1 file changed, 90 insertions(+), 63 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index c246b45..03543dd 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5638,6 +5638,61 @@ static int hvmop_set_evtchn_upcall_vector(
> return 0;
> }
>
> +static int hvm_allow_set_param(struct domain *d,
> + const struct xen_hvm_param *a)
> +{
> + uint64_t value = d->arch.hvm_domain.params[a->index];
> + int rc;
> +
> + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param);
> + if ( rc )
> + return rc;
> +
> + switch ( a->index )
> + {
> + /*
> + * The following parameters must not be set by the guest
> + * since the domain may need to be paused.
> + */
> + case HVM_PARAM_IDENT_PT:
> + case HVM_PARAM_DM_DOMAIN:
> + case HVM_PARAM_ACPI_S_STATE:
> + /* The following parameters should not be set by the guest. */
> + case HVM_PARAM_VIRIDIAN:
> + case HVM_PARAM_MEMORY_EVENT_CR0:
> + case HVM_PARAM_MEMORY_EVENT_CR3:
> + case HVM_PARAM_MEMORY_EVENT_CR4:
> + case HVM_PARAM_MEMORY_EVENT_INT3:
> + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP:
> + case HVM_PARAM_MEMORY_EVENT_MSR:
> + case HVM_PARAM_IOREQ_SERVER_PFN:
> + case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> + if ( d == current->domain )
> + rc = -EPERM;
> + break;
> + default:
> + break;
> + }
> +
> + if ( rc )
> + return rc;
> +
> + switch ( a->index )
> + {
> + /* The following parameters should only be changed once. */
> + case HVM_PARAM_VIRIDIAN:
> + case HVM_PARAM_IOREQ_SERVER_PFN:
> + case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> + if ( value != 0 && a->value != value )
> + rc = -EEXIST;
> + break;
> + default:
> + break;
> + }
> +
> + return rc;
> +}
> +
> static int hvmop_set_param(
> XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg)
> {
> @@ -5662,7 +5717,7 @@ static int hvmop_set_param(
> (is_pvh_domain(d) && (a.index != HVM_PARAM_CALLBACK_IRQ)) )
> goto out;
>
> - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param);
> + rc = hvm_allow_set_param(d, &a);
> if ( rc )
> goto out;
>
> @@ -5677,31 +5732,11 @@ static int hvmop_set_param(
> rc = -EINVAL;
> break;
> case HVM_PARAM_VIRIDIAN:
> - /* This should only ever be set once by the tools and read by the
> guest. */
> - rc = -EPERM;
> - if ( d == curr_d )
> - break;
> -
> - if ( a.value != d->arch.hvm_domain.params[a.index] )
> - {
> - rc = -EEXIST;
> - if ( d->arch.hvm_domain.params[a.index] != 0 )
> - break;
> -
> + if ( (a.value & ~HVMPV_feature_mask) ||
> + !(a.value & HVMPV_base_freq) )
> rc = -EINVAL;
> - if ( (a.value & ~HVMPV_feature_mask) ||
> - !(a.value & HVMPV_base_freq) )
> - break;
> - }
> -
> - rc = 0;
> break;
> case HVM_PARAM_IDENT_PT:
> - /* Not reflexive, as we must domain_pause(). */
> - rc = -EPERM;
> - if ( d == curr_d )
> - break;
> -
> rc = -EINVAL;
> if ( d->arch.hvm_domain.params[a.index] != 0 )
> break;
> @@ -5729,22 +5764,12 @@ static int hvmop_set_param(
> domctl_lock_release();
> break;
> case HVM_PARAM_DM_DOMAIN:
> - /* Not reflexive, as we may need to domain_pause(). */
> - rc = -EPERM;
> - if ( d == curr_d )
> - break;
> -
> if ( a.value == DOMID_SELF )
> a.value = curr_d->domain_id;
>
> rc = hvm_set_dm_domain(d, a.value);
> break;
> case HVM_PARAM_ACPI_S_STATE:
> - /* Not reflexive, as we must domain_pause(). */
> - rc = -EPERM;
> - if ( d == curr_d )
> - break;
> -
> rc = 0;
> if ( a.value == 3 )
> hvm_s3_suspend(d);
> @@ -5757,20 +5782,9 @@ static int hvmop_set_param(
> case HVM_PARAM_ACPI_IOPORTS_LOCATION:
> rc = pmtimer_change_ioport(d, a.value);
> break;
> - case HVM_PARAM_MEMORY_EVENT_CR0:
> - case HVM_PARAM_MEMORY_EVENT_CR3:
> - case HVM_PARAM_MEMORY_EVENT_CR4:
> - if ( d == curr_d )
> - rc = -EPERM;
> - break;
> case HVM_PARAM_MEMORY_EVENT_INT3:
> case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP:
> case HVM_PARAM_MEMORY_EVENT_MSR:
> - if ( d == curr_d )
> - {
> - rc = -EPERM;
> - break;
> - }
> if ( a.value & HVMPME_onchangeonly )
> rc = -EINVAL;
> break;
> @@ -5804,22 +5818,12 @@ static int hvmop_set_param(
> rc = -EINVAL;
> break;
> case HVM_PARAM_IOREQ_SERVER_PFN:
> - if ( d == curr_d )
> - {
> - rc = -EPERM;
> - break;
> - }
> d->arch.hvm_domain.ioreq_gmfn.base = a.value;
> break;
> case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> {
> unsigned int i;
>
> - if ( d == curr_d )
> - {
> - rc = -EPERM;
> - break;
> - }
> if ( a.value == 0 ||
> a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 )
> {
> @@ -5859,10 +5863,40 @@ static int hvmop_set_param(
> return rc;
> }
>
> +static int hvm_allow_get_param(struct domain *d,
> + const struct xen_hvm_param *a)
> +{
> + int rc;
> +
> + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param);
> + if ( rc )
> + return rc;
> +
> + switch ( a->index )
> + {
> + /*
> + * The following parameters must not be read by the guest
> + * since the domain may need to be paused.
> + */
> + case HVM_PARAM_IOREQ_PFN:
> + case HVM_PARAM_BUFIOREQ_PFN:
> + case HVM_PARAM_BUFIOREQ_EVTCHN:
> + /* The following parameters should not be read by the guest. */
> + case HVM_PARAM_IOREQ_SERVER_PFN:
> + case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> + if ( d == current->domain )
> + rc = -EPERM;
> + break;
> + default:
> + break;
> + }
> +
> + return rc;
> +}
> +
> static int hvmop_get_param(
> XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg)
> {
> - struct domain *curr_d = current->domain;
> struct xen_hvm_param a;
> struct domain *d;
> int rc;
> @@ -5882,7 +5916,7 @@ static int hvmop_get_param(
> (is_pvh_domain(d) && (a.index != HVM_PARAM_CALLBACK_IRQ)) )
> goto out;
>
> - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param);
> + rc = hvm_allow_get_param(d, &a);
> if ( rc )
> goto out;
>
> @@ -5891,13 +5925,6 @@ static int hvmop_get_param(
> case HVM_PARAM_ACPI_S_STATE:
> a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0;
> break;
> - case HVM_PARAM_IOREQ_SERVER_PFN:
> - case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> - if ( d == curr_d )
> - {
> - rc = -EPERM;
> - goto out;
> - }
> case HVM_PARAM_IOREQ_PFN:
> case HVM_PARAM_BUFIOREQ_PFN:
> case HVM_PARAM_BUFIOREQ_EVTCHN:
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |