[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 07/19] xen: avoid calling rcu_lock_*target_domain when an XSM hook exists
The rcu_lock_{,remote_}target_domain_by_id functions are wrappers around an IS_PRIV_FOR check for the current domain. This is now redundant with XSM hooks, so replace these calls with rcu_lock_domain_by_any_id or rcu_lock_remote_domain_by_id to remove the duplicate permission checks. When XSM_ENABLE is not defined or when the dummy XSM module is used, this patch should not change any functionality. Because the locations of privilege checks have sometimes moved below argument validation, error returns of some functions may change from EPERM to EINVAL when called with invalid arguments and from a domain without permission to perform the operation. Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> Cc: Keir Fraser <keir@xxxxxxx> Cc: Jan Beulich <jbeulich@xxxxxxxx> --- xen/arch/x86/hvm/hvm.c | 38 +++++++++++++++---------------- xen/arch/x86/mm.c | 22 ++++++++---------- xen/common/event_channel.c | 18 +++++++-------- xen/common/grant_table.c | 57 +++++++++++++++------------------------------- xen/common/memory.c | 15 ++++++------ xen/include/xsm/dummy.h | 34 +++++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 87 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 34da2f5..5bdde8d 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3392,7 +3392,7 @@ static int hvmop_set_pci_intx_level( if ( (op.domain > 0) || (op.bus > 0) || (op.device > 31) || (op.intx > 3) ) return -EINVAL; - rc = rcu_lock_remote_target_domain_by_id(op.domid, &d); + rc = rcu_lock_remote_domain_by_id(op.domid, &d); if ( rc != 0 ) return rc; @@ -3559,7 +3559,7 @@ static int hvmop_set_isa_irq_level( if ( op.isa_irq > 15 ) return -EINVAL; - rc = rcu_lock_remote_target_domain_by_id(op.domid, &d); + rc = rcu_lock_remote_domain_by_id(op.domid, &d); if ( rc != 0 ) return rc; @@ -3603,7 +3603,7 @@ static int hvmop_set_pci_link_route( if ( (op.link > 3) || (op.isa_irq > 15) ) return -EINVAL; - rc = rcu_lock_remote_target_domain_by_id(op.domid, &d); + rc = rcu_lock_remote_domain_by_id(op.domid, &d); if ( rc != 0 ) return rc; @@ -3633,7 +3633,7 @@ static int hvmop_inject_msi( if ( copy_from_guest(&op, uop, 1) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(op.domid, &d); + rc = rcu_lock_remote_domain_by_id(op.domid, &d); if ( rc != 0 ) return rc; @@ -3730,9 +3730,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( a.index >= HVM_NR_PARAMS ) return -EINVAL; - rc = rcu_lock_target_domain_by_id(a.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; rc = -EINVAL; if ( !is_hvm_domain(d) ) @@ -3976,7 +3976,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(a.domid, &d); + rc = rcu_lock_remote_domain_by_id(a.domid, &d); if ( rc != 0 ) return rc; @@ -4015,7 +4015,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(a.domid, &d); + rc = rcu_lock_remote_domain_by_id(a.domid, &d); if ( rc != 0 ) return rc; @@ -4065,9 +4065,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(a.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; rc = xsm_hvm_param(d, op); if ( rc ) @@ -4116,7 +4116,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(a.domid, &d); + rc = rcu_lock_remote_domain_by_id(a.domid, &d); if ( rc != 0 ) return rc; @@ -4195,7 +4195,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(a.domid, &d); + rc = rcu_lock_remote_domain_by_id(a.domid, &d); if ( rc != 0 ) return rc; @@ -4230,7 +4230,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(a.domid, &d); + rc = rcu_lock_remote_domain_by_id(a.domid, &d); if ( rc != 0 ) return rc; @@ -4266,9 +4266,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&a, arg, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(a.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; rc = -EINVAL; if ( !is_hvm_domain(d) || !paging_mode_shadow(d) ) @@ -4320,7 +4320,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&tr, arg, 1 ) ) return -EFAULT; - rc = rcu_lock_remote_target_domain_by_id(tr.domid, &d); + rc = rcu_lock_remote_domain_by_id(tr.domid, &d); if ( rc != 0 ) return rc; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 9c90067..b065bcf 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4389,9 +4389,9 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&xatp, arg, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(xatp.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(xatp.domid); + if ( d == NULL ) + return -ESRCH; if ( xsm_add_to_physmap(current->domain, d) ) { @@ -4428,9 +4428,9 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( fmap.map.nr_entries > E820MAX ) return -EINVAL; - rc = rcu_lock_target_domain_by_id(fmap.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(fmap.domid); + if ( d == NULL ) + return -ESRCH; rc = xsm_domain_memory_map(d); if ( rc ) @@ -4583,16 +4583,12 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) struct domain *d; struct p2m_domain *p2m; - /* Support DOMID_SELF? */ - if ( !IS_PRIV(current->domain) ) - return -EPERM; - if ( copy_from_guest(&target, arg, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(target.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(target.domid); + if ( d == NULL ) + return -ESRCH; if ( op == XENMEM_set_pod_target ) rc = xsm_set_pod_target(d); diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c index a80a0d1..37947a9 100644 --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -165,9 +165,9 @@ static long evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc) domid_t dom = alloc->dom; long rc; - rc = rcu_lock_target_domain_by_id(dom, &d); - if ( rc ) - return rc; + d = rcu_lock_domain_by_any_id(dom); + if ( d == NULL ) + return -ESRCH; spin_lock(&d->event_lock); @@ -798,9 +798,9 @@ static long evtchn_status(evtchn_status_t *status) struct evtchn *chn; long rc = 0; - rc = rcu_lock_target_domain_by_id(dom, &d); - if ( rc ) - return rc; + d = rcu_lock_domain_by_any_id(dom); + if ( d == NULL ) + return -ESRCH; spin_lock(&d->event_lock); @@ -950,9 +950,9 @@ static long evtchn_reset(evtchn_reset_t *r) struct domain *d; int i, rc; - rc = rcu_lock_target_domain_by_id(dom, &d); - if ( rc ) - return rc; + d = rcu_lock_domain_by_any_id(dom); + if ( d == NULL ) + return -ESRCH; rc = xsm_evtchn_reset(current->domain, d); if ( rc ) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 7912769..eae9518 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -230,30 +230,6 @@ double_gt_unlock(struct grant_table *lgt, struct grant_table *rgt) spin_unlock(&rgt->lock); } -static struct domain *gt_lock_target_domain_by_id(domid_t dom) -{ - struct domain *d; - int rc = GNTST_general_error; - - switch ( rcu_lock_target_domain_by_id(dom, &d) ) - { - case 0: - return d; - - case -ESRCH: - gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom); - rc = GNTST_bad_domain; - break; - - case -EPERM: - rc = GNTST_permission_denied; - break; - } - - ASSERT(rc < 0 && -rc <= MAX_ERRNO); - return ERR_PTR(rc); -} - static inline int __get_maptrack_handle( struct grant_table *t) @@ -1342,11 +1318,12 @@ gnttab_setup_table( goto out1; } - d = gt_lock_target_domain_by_id(op.dom); - if ( IS_ERR(d) ) + d = rcu_lock_domain_by_any_id(op.dom); + if ( d == NULL ) { - op.status = PTR_ERR(d); - goto out1; + gdprintk(XENLOG_INFO, "Bad domid %d.\n", op.dom); + op.status = GNTST_bad_domain; + goto out2; } if ( xsm_grant_setup(current->domain, d) ) @@ -1410,10 +1387,11 @@ gnttab_query_size( return -EFAULT; } - d = gt_lock_target_domain_by_id(op.dom); - if ( IS_ERR(d) ) + d = rcu_lock_domain_by_any_id(op.dom); + if ( d == NULL ) { - op.status = PTR_ERR(d); + gdprintk(XENLOG_INFO, "Bad domid %d.\n", op.dom); + op.status = GNTST_bad_domain; goto query_out; } @@ -2283,10 +2261,10 @@ gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop, return -EFAULT; } - d = gt_lock_target_domain_by_id(op.dom); - if ( IS_ERR(d) ) + d = rcu_lock_domain_by_any_id(op.dom); + if ( d == NULL ) { - op.status = PTR_ERR(d); + op.status = GNTST_bad_domain; goto out1; } rc = xsm_grant_setup(current->domain, d); @@ -2336,14 +2314,15 @@ gnttab_get_version(XEN_GUEST_HANDLE_PARAM(gnttab_get_version_t uop)) if ( copy_from_guest(&op, uop, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(op.dom, &d); - if ( rc < 0 ) - return rc; + d = rcu_lock_domain_by_any_id(op.dom); + if ( d == NULL ) + return -ESRCH; - if ( xsm_grant_query_size(current->domain, d) ) + rc = xsm_grant_query_size(current->domain, d); + if ( rc ) { rcu_unlock_domain(d); - return -EPERM; + return rc; } op.version = d->grant_table->gt_version; diff --git a/xen/common/memory.c b/xen/common/memory.c index 83e2666..dd36b83 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -570,7 +570,8 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) && (reservation.mem_flags & XENMEMF_populate_on_demand) ) args.memflags |= MEMF_populate_on_demand; - if ( unlikely(rcu_lock_target_domain_by_id(reservation.domid, &d)) ) + d = rcu_lock_domain_by_any_id(reservation.domid); + if ( d == NULL ) return start_extent; args.domain = d; @@ -619,9 +620,9 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&domid, arg, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(domid, &d); - if ( rc ) - return rc; + d = rcu_lock_domain_by_any_id(domid); + if ( d == NULL ) + return -ESRCH; rc = xsm_memory_stat_reservation(current->domain, d); if ( rc ) @@ -657,9 +658,9 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&xrfp, arg, 1) ) return -EFAULT; - rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(xrfp.domid); + if ( d == NULL ) + return -ESRCH; if ( xsm_remove_from_physmap(current->domain, d) ) { diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 12b1508..17128f5 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -194,6 +194,8 @@ static XSM_DEFAULT(int, grant_unmapref) (struct domain *d1, struct domain *d2) static XSM_DEFAULT(int, grant_setup) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } @@ -209,17 +211,23 @@ static XSM_DEFAULT(int, grant_copy) (struct domain *d1, struct domain *d2) static XSM_DEFAULT(int, grant_query_size) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, memory_adjust_reservation) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, memory_stat_reservation) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } @@ -260,6 +268,8 @@ static XSM_DEFAULT(int, memory_pin_page) (struct domain *d1, struct domain *d2, static XSM_DEFAULT(int, evtchn_unbound) (struct domain *d, struct evtchn *chn, domid_t id2) { + if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } @@ -281,11 +291,15 @@ static XSM_DEFAULT(int, evtchn_send) (struct domain *d, struct evtchn *chn) static XSM_DEFAULT(int, evtchn_status) (struct domain *d, struct evtchn *chn) { + if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, evtchn_reset) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } @@ -306,11 +320,15 @@ static XSM_DEFAULT(char *, show_security_evtchn) (struct domain *d, const struct static XSM_DEFAULT(int, get_pod_target)(struct domain *d) { + if ( !IS_PRIV(current->domain) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, set_pod_target)(struct domain *d) { + if ( !IS_PRIV(current->domain) ) + return -EPERM; return 0; } @@ -481,26 +499,36 @@ static XSM_DEFAULT(int, machine_address_size) (struct domain *d, uint32_t cmd) static XSM_DEFAULT(int, hvm_param) (struct domain *d, unsigned long op) { + if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, hvm_set_pci_intx_level) (struct domain *d) { + if ( !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, hvm_set_isa_irq_level) (struct domain *d) { + if ( !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, hvm_set_pci_link_route) (struct domain *d) { + if ( !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, hvm_inject_msi) (struct domain *d) { + if ( !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } @@ -582,6 +610,8 @@ static XSM_DEFAULT(int, machine_memory_map) (void) static XSM_DEFAULT(int, domain_memory_map) (struct domain *d) { + if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) ) + return -EPERM; return 0; } @@ -605,11 +635,15 @@ static XSM_DEFAULT(int, update_va_mapping) (struct domain *d, struct domain *f, static XSM_DEFAULT(int, add_to_physmap) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } static XSM_DEFAULT(int, remove_from_physmap) (struct domain *d1, struct domain *d2) { + if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) ) + return -EPERM; return 0; } -- 1.7.11.7 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |