From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> Introduce use of a virDomainDefPtr in the domain lookup APIs to simplify introduction of ACL security checks. The virDomainPtr cannot be safely used, since the app may have supplied mis-matching name/uuid/id fields. eg the name points to domain X, while the uuid points to domain Y. Resolving the virDomainPtr to a virDomainDefPtr ensures a consistent name/uuid/id set. Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> --- src/conf/domain_conf.c | 24 ++++++++ src/conf/domain_conf.h | 4 ++ src/libvirt_private.syms | 1 + src/xen/xen_driver.c | 147 +++++++++++++++++++++++++++++++---------------- src/xen/xen_hypervisor.c | 17 +++--- src/xen/xen_hypervisor.h | 8 +-- src/xen/xen_inotify.c | 14 ++--- src/xen/xend_internal.c | 34 +++++------ src/xen/xend_internal.h | 4 +- src/xen/xm_internal.c | 30 ++++------ src/xen/xm_internal.h | 5 +- 11 files changed, 173 insertions(+), 115 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d55ce6b..61995cd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2048,6 +2048,30 @@ error: return NULL; } + +virDomainDefPtr virDomainDefNew(const char *name, + const unsigned char *uuid, + int id) +{ + virDomainDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (!(def->name = strdup(name))) { + VIR_FREE(def); + return NULL; + } + + memcpy(def->uuid, uuid, VIR_UUID_BUFLEN); + def->id = id; + + return def; +} + + void virDomainObjAssignDef(virDomainObjPtr domain, const virDomainDefPtr def, bool live, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 21f7ce2..f7644a6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2147,6 +2147,10 @@ void virDomainDefFree(virDomainDefPtr vm); virDomainChrDefPtr virDomainChrDefNew(void); +virDomainDefPtr virDomainDefNew(const char *name, + const unsigned char *uuid, + int id); + enum { VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 << 0), VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE = (1 << 1), diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bb70595..d2f5827 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -117,6 +117,7 @@ virDomainDefGenSecurityLabelDef; virDomainDefGetDefaultEmulator; virDomainDefGetSecurityLabelDef; virDomainDefMaybeAddController; +virDomainDefNew; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index cc54f7a..d9420d8 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -82,6 +82,60 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom, static bool is_privileged = false; +static virDomainDefPtr xenGetDomainDefForID(virConnectPtr conn, int id) +{ + virDomainDefPtr ret; + + ret = xenHypervisorLookupDomainByID(conn, id); + + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; +} + + +static virDomainDefPtr xenGetDomainDefForName(virConnectPtr conn, const char *name) +{ + xenUnifiedPrivatePtr priv = conn->privateData; + virDomainDefPtr ret; + + ret = xenDaemonLookupByName(conn, name); + + /* Try XM for inactive domains. */ + if (!ret && + priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + ret = xenXMDomainLookupByName(conn, name); + + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; +} + + +static virDomainDefPtr xenGetDomainDefForUUID(virConnectPtr conn, const unsigned char *uuid) +{ + xenUnifiedPrivatePtr priv = conn->privateData; + virDomainDefPtr ret; + + ret = xenHypervisorLookupDomainByUUID(conn, uuid); + + /* Try XM for inactive domains. */ + if (!ret) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + ret = xenXMDomainLookupByUUID(conn, uuid); + else + ret = xenDaemonLookupByUUID(conn, uuid); + } + + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; +} + + /** * xenNumaInit: * @conn: pointer to the hypervisor connection @@ -597,12 +651,18 @@ static virDomainPtr xenUnifiedDomainLookupByID(virConnectPtr conn, int id) { virDomainPtr ret = NULL; + virDomainDefPtr def = NULL; - ret = xenHypervisorLookupDomainByID(conn, id); + if (!(def = xenGetDomainDefForID(conn, id))) + goto cleanup; - if (!ret && virGetLastError() == NULL) - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + if (!(ret = virGetDomain(conn, def->name, def->uuid))) + goto cleanup; + ret->id = def->id; + +cleanup: + virDomainDefFree(def); return ret; } @@ -610,22 +670,19 @@ static virDomainPtr xenUnifiedDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainPtr ret = NULL; + virDomainDefPtr def = NULL; - ret = xenHypervisorLookupDomainByUUID(conn, uuid); + if (!(def = xenGetDomainDefForUUID(conn, uuid))) + goto cleanup; - /* Try XM for inactive domains. */ - if (!ret) { - if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) - ret = xenXMDomainLookupByUUID(conn, uuid); - else - ret = xenDaemonLookupByUUID(conn, uuid); - } + if (!(ret = virGetDomain(conn, def->name, def->uuid))) + goto cleanup; - if (!ret && virGetLastError() == NULL) - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + ret->id = def->id; +cleanup: + virDomainDefFree(def); return ret; } @@ -633,18 +690,19 @@ static virDomainPtr xenUnifiedDomainLookupByName(virConnectPtr conn, const char *name) { - xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainPtr ret = NULL; + virDomainDefPtr def = NULL; - ret = xenDaemonLookupByName(conn, name); + if (!(def = xenGetDomainDefForName(conn, name))) + goto cleanup; - /* Try XM for inactive domains. */ - if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) - ret = xenXMDomainLookupByName(conn, name); + if (!(ret = virGetDomain(conn, def->name, def->uuid))) + goto cleanup; - if (!ret && virGetLastError() == NULL) - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + ret->id = def->id; +cleanup: + virDomainDefFree(def); return ret; } @@ -652,28 +710,18 @@ xenUnifiedDomainLookupByName(virConnectPtr conn, static int xenUnifiedDomainIsActive(virDomainPtr dom) { - xenUnifiedPrivatePtr priv = dom->conn->privateData; - virDomainPtr currdom; + virDomainDefPtr def; int ret = -1; - /* ID field in dom may be outdated, so re-lookup */ - currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); - - /* Try XM for inactive domains. */ - if (!currdom) { - if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) - currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid); - else - currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid); - } + if (!(def = xenGetDomainDefForUUID(dom->conn, dom->uuid))) + goto cleanup; - if (currdom) { - ret = currdom->id == -1 ? 0 : 1; - virDomainFree(currdom); - } else if (virGetLastError() == NULL) { - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + if (def) { + ret = def->id == -1 ? 0 : 1; + virDomainDefFree(def); } +cleanup: return ret; } @@ -681,22 +729,22 @@ static int xenUnifiedDomainIsPersistent(virDomainPtr dom) { xenUnifiedPrivatePtr priv = dom->conn->privateData; - virDomainPtr currdom = NULL; + virDomainDefPtr def = NULL; int ret = -1; if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { /* Old Xen, pre-inactive domain management. * If the XM driver can see the guest, it is definitely persistent */ - currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid); - if (currdom) + def = xenXMDomainLookupByUUID(dom->conn, dom->uuid); + if (def) ret = 1; else ret = 0; } else { /* New Xen with inactive domain management */ - currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid); - if (currdom) { - if (currdom->id == -1) { + def = xenDaemonLookupByUUID(dom->conn, dom->uuid); + if (def) { + if (def->id == -1) { /* If its inactive, then trivially, it must be persistent */ ret = 1; } else { @@ -708,7 +756,7 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom) virUUIDFormat(dom->uuid, uuidstr); if (virAsprintf(&path, "%s/%s", XEND_DOMAINS_DIR, uuidstr) < 0) { virReportOOMError(); - goto done; + goto cleanup; } if (access(path, R_OK) == 0) ret = 1; @@ -718,9 +766,8 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom) } } -done: - if (currdom) - virDomainFree(currdom); +cleanup: + virDomainDefFree(def); return ret; } diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 68a3ef4..55b0930 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2562,12 +2562,13 @@ xenHypervisorHasDomain(virConnectPtr conn, int id) return 1; } -virDomainPtr + +virDomainDefPtr xenHypervisorLookupDomainByID(virConnectPtr conn, int id) { xenUnifiedPrivatePtr priv = conn->privateData; xen_getdomaininfo dominfo; - virDomainPtr ret; + virDomainDefPtr ret; char *name; XEN_GETDOMAININFO_CLEAR(dominfo); @@ -2584,20 +2585,20 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id) if (!name) return NULL; - ret = virGetDomain(conn, name, XEN_GETDOMAININFO_UUID(dominfo)); - if (ret) - ret->id = id; + ret = virDomainDefNew(name, + XEN_GETDOMAININFO_UUID(dominfo), + id); VIR_FREE(name); return ret; } -virDomainPtr +virDomainDefPtr xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { xen_getdomaininfolist dominfos; xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainDefPtr ret; char *name; int maxids = 100, nids, i, id; @@ -2648,7 +2649,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) if (!name) return NULL; - ret = virGetDomain(conn, name, uuid); + ret = virDomainDefNew(name, uuid, id); if (ret) ret->id = id; VIR_FREE(name); diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 8507bd0..1d44a92 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -27,6 +27,7 @@ # include "capabilities.h" # include "driver.h" # include "viruri.h" +# include "domain_conf.h" /* See xenHypervisorInit() for details. */ struct xenHypervisorVersions { @@ -43,10 +44,9 @@ virCapsPtr xenHypervisorMakeCapabilities (virConnectPtr conn); int xenHypervisorHasDomain(virConnectPtr conn, int id); -virDomainPtr - xenHypervisorLookupDomainByID (virConnectPtr conn, - int id); -virDomainPtr +virDomainDefPtr + xenHypervisorLookupDomainByID (virConnectPtr conn, int id); +virDomainDefPtr xenHypervisorLookupDomainByUUID (virConnectPtr conn, const unsigned char *uuid); char * diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index b032bba..576eb10 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -76,7 +76,7 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, unsigned char *uuid) { int i; - virDomainPtr dom; + virDomainDefPtr def; const char *uuid_str; unsigned char rawuuid[VIR_UUID_BUFLEN]; xenUnifiedPrivatePtr priv = conn->privateData; @@ -96,8 +96,8 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, be set during open while we are building our initial list of domains */ VIR_DEBUG("Looking for dom with uuid: %s", uuid_str); - /* XXX Should not have to go via a virDomainPtr obj instance */ - if (!(dom = xenDaemonLookupByUUID(conn, rawuuid))) { + + if (!(def = xenDaemonLookupByUUID(conn, rawuuid))) { /* If we are here, the domain has gone away. search for, and create a domain from the stored list info */ @@ -118,13 +118,13 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, return -1; } - if (!(*name = strdup(dom->name))) { + if (!(*name = strdup(def->name))) { virReportOOMError(); - virDomainFree(dom); + virDomainDefFree(def); return -1; } - memcpy(uuid, dom->uuid, VIR_UUID_BUFLEN); - virDomainFree(dom); + memcpy(uuid, def->uuid, VIR_UUID_BUFLEN); + virDomainDefFree(def); /* succeeded too find domain by uuid */ return 0; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index aec57f5..5ea1627 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1111,15 +1111,16 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) * * Internal routine returning the associated virDomainPtr for this domain * - * Returns the domain pointer or NULL in case of error. + * Returns the domain def pointer or NULL in case of error. */ -static virDomainPtr +static virDomainDefPtr sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) { - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; const char *name; const char *tmp; + int id = -1; xenUnifiedPrivatePtr priv = conn->privateData; if (sexpr_uuid(uuid, root, "domain/uuid") < 0) @@ -1128,9 +1129,6 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) if (name == NULL) goto error; - ret = virGetDomain(conn, name, uuid); - if (ret == NULL) return NULL; - tmp = sexpr_node(root, "domain/domid"); /* New 3.0.4 XenD will not report a domid for inactive domains, * so only error out for old XenD @@ -1139,11 +1137,9 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) goto error; if (tmp) - ret->id = sexpr_int(root, "domain/domid"); - else - ret->id = -1; /* An inactive domain */ + id = sexpr_int(root, "domain/domid"); - return ret; + return virDomainDefNew(name, uuid, id); error: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1694,13 +1690,13 @@ xenDaemonDomainGetState(virDomainPtr domain, * it in the form of a struct xend_domain. This should be * free()'d when no longer needed. * - * Returns domain info on success; NULL (with errno) on error + * Returns domain def pointer on success; NULL on error */ -virDomainPtr +virDomainDefPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname) { struct sexpr *root; - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; root = sexpr_get(conn, "/xend/domain/%s?detail=1", domname); if (root == NULL) @@ -2048,12 +2044,12 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, * * Try to lookup a domain on xend based on its UUID. * - * Returns a new domain object or NULL in case of failure + * Returns domain def pointer on success; NULL on error */ -virDomainPtr +virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - virDomainPtr ret; + virDomainDefPtr ret; char *name = NULL; int id = -1; xenUnifiedPrivatePtr priv = conn->privateData; @@ -2113,12 +2109,8 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (name == NULL) return NULL; - ret = virGetDomain(conn, name, uuid); - if (ret == NULL) goto cleanup; - - ret->id = id; + ret = virDomainDefNew(name, uuid, id); - cleanup: VIR_FREE(name); return ret; } diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 7332303..a2d05f3 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -149,8 +149,8 @@ int xenDaemonDomainSetAutostart (virDomainPtr domain, int autostart); virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc); -virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); -virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); +virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); +virDomainDefPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); int xenDaemonDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cookielen, const char *uri_in, char **uri_out, unsigned long flags, const char *dname, unsigned long resource); int xenDaemonDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long resource); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 66bd289..9b8d742 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -814,13 +814,13 @@ xenXMDomainPinVcpu(virDomainPtr domain, /* * Find an inactive domain based on its name */ -virDomainPtr +virDomainDefPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname) { xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; xenUnifiedLock(priv); @@ -833,12 +833,7 @@ xenXMDomainLookupByName(virConnectPtr conn, const char *domname) if (!(entry = virHashLookup(priv->configCache, filename))) goto cleanup; - if (!(ret = virGetDomain(conn, domname, entry->def->uuid))) - goto cleanup; - - /* Ensure its marked inactive, because may be cached - handle to a previously active domain */ - ret->id = -1; + ret = virDomainDefNew(domname, entry->def->uuid, -1); cleanup: xenUnifiedUnlock(priv); @@ -866,12 +861,12 @@ xenXMDomainSearchForUUID(const void *payload, /* * Find an inactive domain based on its UUID */ -virDomainPtr +virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; - virDomainPtr ret = NULL; + virDomainDefPtr ret = NULL; xenUnifiedLock(priv); @@ -881,12 +876,7 @@ xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, (const void *)uuid))) goto cleanup; - if (!(ret = virGetDomain(conn, entry->def->name, uuid))) - goto cleanup; - - /* Ensure its marked inactive, because may be cached - handle to a previously active domain */ - ret->id = -1; + ret = virDomainDefNew(entry->def->name, uuid, -1); cleanup: xenUnifiedUnlock(priv); @@ -1129,7 +1119,7 @@ struct xenXMListIteratorContext { static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { struct xenXMListIteratorContext *ctx = data; - virDomainPtr dom = NULL; + virDomainDefPtr def = NULL; if (ctx->oom) return; @@ -1137,14 +1127,14 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) if (ctx->count == ctx->max) return; - dom = xenDaemonLookupByName(ctx->conn, name); - if (!dom) { + def = xenDaemonLookupByName(ctx->conn, name); + if (!def) { if (!(ctx->names[ctx->count] = strdup(name))) ctx->oom = 1; else ctx->count++; } else { - virDomainFree(dom); + virDomainDefFree(def); } } diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index eda7394..0e55897 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -51,9 +51,8 @@ int xenXMDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, int xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags); int xenXMDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, int maplen); -virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname); -virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid); +virDomainDefPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname); +virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid); int xenXMListDefinedDomains(virConnectPtr conn, char ** const names, int maxnames); int xenXMNumOfDefinedDomains(virConnectPtr conn); -- 1.8.1.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list