Message ID | 20200219193001.28822-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xen/arm: Restrict access to most HVM_PARAM's | expand |
Hi Andrew, On 19/02/2020 19:30, Andrew Cooper wrote: > ARM currently has no restrictions on toolstack and guest access to the entire > HVM_PARAM block. As the monitor feature isn't under security support, this > doesn't need an XSA. > > The CALLBACK_IRQ and {STORE,CONSOLE}_{PFN,EVTCHN} details are only exposed > read-only to the guest, while MONITOR_RING_PFN is restricted to only toolstack > access. No other parameters are used. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Julien Grall <julien@xen.org> Cheers, > --- > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien@xen.org> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > > This is only compile tested, and based on my reading of the source. There > might be other PARAMS needing including. > > v2: > * Drop paging/sharing > * Clarify comment about {STORE,CONSOLE}_EVTCHN > --- > xen/arch/arm/hvm.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 59 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > index 76b27c9168..8951b34086 100644 > --- a/xen/arch/arm/hvm.c > +++ b/xen/arch/arm/hvm.c > @@ -31,6 +31,57 @@ > > #include <asm/hypercall.h> > > +static int hvm_allow_set_param(const struct domain *d, unsigned int param) > +{ > + switch ( param ) > + { > + /* > + * The following parameters are intended for toolstack usage only. > + * They may not be set by the domain. > + * > + * The {STORE,CONSOLE}_EVTCHN values will need to become read/write to > + * the guest (not just the toolstack) if a new ABI hasn't appeared by > + * the time migration support is added. > + */ > + case HVM_PARAM_CALLBACK_IRQ: > + case HVM_PARAM_STORE_PFN: > + case HVM_PARAM_STORE_EVTCHN: > + case HVM_PARAM_CONSOLE_PFN: > + case HVM_PARAM_CONSOLE_EVTCHN: > + case HVM_PARAM_MONITOR_RING_PFN: > + return d == current->domain ? -EPERM : 0; > + > + /* Writeable only by Xen, hole, deprecated, or out-of-range. */ > + default: > + return -EINVAL; > + } > +} > + > +static int hvm_allow_get_param(const struct domain *d, unsigned int param) > +{ > + switch ( param ) > + { > + /* The following parameters can be read by the guest and toolstack. */ > + case HVM_PARAM_CALLBACK_IRQ: > + case HVM_PARAM_STORE_PFN: > + case HVM_PARAM_STORE_EVTCHN: > + case HVM_PARAM_CONSOLE_PFN: > + case HVM_PARAM_CONSOLE_EVTCHN: > + return 0; > + > + /* > + * The following parameters are intended for toolstack usage only. > + * They may not be read by the domain. > + */ > + case HVM_PARAM_MONITOR_RING_PFN: > + return d == current->domain ? -EPERM : 0; > + > + /* Hole, deprecated, or out-of-range. */ > + default: > + return -EINVAL; > + } > +} > + > long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > { > long rc = 0; > @@ -46,9 +97,6 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > if ( copy_from_guest(&a, arg, 1) ) > return -EFAULT; > > - if ( a.index >= HVM_NR_PARAMS ) > - return -EINVAL; > - > d = rcu_lock_domain_by_any_id(a.domid); > if ( d == NULL ) > return -ESRCH; > @@ -59,10 +107,18 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > > if ( op == HVMOP_set_param ) > { > + rc = hvm_allow_set_param(d, a.index); > + if ( rc ) > + goto param_fail; > + > d->arch.hvm.params[a.index] = a.value; > } > else > { > + rc = hvm_allow_get_param(d, a.index); > + if ( rc ) > + goto param_fail; > + > a.value = d->arch.hvm.params[a.index]; > rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > } >
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c index 76b27c9168..8951b34086 100644 --- a/xen/arch/arm/hvm.c +++ b/xen/arch/arm/hvm.c @@ -31,6 +31,57 @@ #include <asm/hypercall.h> +static int hvm_allow_set_param(const struct domain *d, unsigned int param) +{ + switch ( param ) + { + /* + * The following parameters are intended for toolstack usage only. + * They may not be set by the domain. + * + * The {STORE,CONSOLE}_EVTCHN values will need to become read/write to + * the guest (not just the toolstack) if a new ABI hasn't appeared by + * the time migration support is added. + */ + case HVM_PARAM_CALLBACK_IRQ: + case HVM_PARAM_STORE_PFN: + case HVM_PARAM_STORE_EVTCHN: + case HVM_PARAM_CONSOLE_PFN: + case HVM_PARAM_CONSOLE_EVTCHN: + case HVM_PARAM_MONITOR_RING_PFN: + return d == current->domain ? -EPERM : 0; + + /* Writeable only by Xen, hole, deprecated, or out-of-range. */ + default: + return -EINVAL; + } +} + +static int hvm_allow_get_param(const struct domain *d, unsigned int param) +{ + switch ( param ) + { + /* The following parameters can be read by the guest and toolstack. */ + case HVM_PARAM_CALLBACK_IRQ: + case HVM_PARAM_STORE_PFN: + case HVM_PARAM_STORE_EVTCHN: + case HVM_PARAM_CONSOLE_PFN: + case HVM_PARAM_CONSOLE_EVTCHN: + return 0; + + /* + * The following parameters are intended for toolstack usage only. + * They may not be read by the domain. + */ + case HVM_PARAM_MONITOR_RING_PFN: + return d == current->domain ? -EPERM : 0; + + /* Hole, deprecated, or out-of-range. */ + default: + return -EINVAL; + } +} + long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) { long rc = 0; @@ -46,9 +97,6 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - if ( a.index >= HVM_NR_PARAMS ) - return -EINVAL; - d = rcu_lock_domain_by_any_id(a.domid); if ( d == NULL ) return -ESRCH; @@ -59,10 +107,18 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( op == HVMOP_set_param ) { + rc = hvm_allow_set_param(d, a.index); + if ( rc ) + goto param_fail; + d->arch.hvm.params[a.index] = a.value; } else { + rc = hvm_allow_get_param(d, a.index); + if ( rc ) + goto param_fail; + a.value = d->arch.hvm.params[a.index]; rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; }
ARM currently has no restrictions on toolstack and guest access to the entire HVM_PARAM block. As the monitor feature isn't under security support, this doesn't need an XSA. The CALLBACK_IRQ and {STORE,CONSOLE}_{PFN,EVTCHN} details are only exposed read-only to the guest, while MONITOR_RING_PFN is restricted to only toolstack access. No other parameters are used. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> This is only compile tested, and based on my reading of the source. There might be other PARAMS needing including. v2: * Drop paging/sharing * Clarify comment about {STORE,CONSOLE}_EVTCHN --- xen/arch/arm/hvm.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 3 deletions(-)