When using XSM Flask, passing DOMIND_INVALID will result in a NULL pointer reference from the passing of NULL as the target domain to xsm_get_domain_state(). Simply not invoking xsm_get_domain_state() when the target domain is NULL opens the opportunity to circumvent the XSM get_domain_state access check. This is due to the fact that the call to xsm_domctl() for get_domain_state op is a no-op check, deferring to xsm_get_domain_state().
Modify the helper get_domain_state() to ensure the requesting domain has get_domain_state access for the target domain, whether the target domain is explicitly set or implicitly determined with a domain state search. In the case of access not being allowed for a domain found during an implicit search, the search will continue to the next domain whose state has changed. Signed-off-by: Daniel P. Smith <[email protected]> Reported-by: Chris Rogers <[email protected]> Fixes: 3ad3df1bd0aa ("xen: add new domctl get_domain_state") --- xen/common/domain.c | 22 +++++++++++++++++++--- xen/common/domctl.c | 7 ++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/xen/common/domain.c b/xen/common/domain.c index de6fdf59236e..4886c59c874c 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -210,7 +210,7 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, domid_t *domid) { - unsigned int dom; + unsigned int dom = 0; int rc = -ENOENT; struct domain *hdl; @@ -219,6 +219,10 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, if ( d ) { + rc = xsm_get_domain_state(XSM_XS_PRIV, d); + if ( rc ) + return rc; + set_domain_state_info(info, d); return 0; @@ -238,10 +242,10 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, while ( dom_state_changed ) { - dom = find_first_bit(dom_state_changed, DOMID_MASK + 1); + dom = find_next_bit(dom_state_changed, DOMID_MASK + 1, dom); if ( dom >= DOMID_FIRST_RESERVED ) break; - if ( test_and_clear_bit(dom, dom_state_changed) ) + if ( test_bit(dom, dom_state_changed) ) { *domid = dom; @@ -249,6 +253,15 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, if ( d ) { + rc = xsm_get_domain_state(XSM_XS_PRIV, d); + if ( rc ) + { + rcu_unlock_domain(d); + rc = -ENOENT; + dom++; + continue; + } + set_domain_state_info(info, d); rcu_unlock_domain(d); @@ -256,10 +269,13 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, else memset(info, 0, sizeof(*info)); + clear_bit(dom, dom_state_changed); rc = 0; break; } + + dom++; } out: diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 29a7726d32d0..2eedc639c72a 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -860,12 +860,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) break; case XEN_DOMCTL_get_domain_state: - ret = xsm_get_domain_state(XSM_XS_PRIV, d); - if ( ret ) - break; - - copyback = 1; ret = get_domain_state(&op->u.get_domain_state, d, &op->domain); + if ( !ret ) + copyback = 1; break; default: -- 2.39.5
