Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Introduce use of a virDomainDefPtr in the domain stats &
> peek 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/xen/block_stats.c | 6 +++---
> src/xen/block_stats.h | 2 +-
> src/xen/xen_driver.c | 37 +++++++++++++++++++++++++++++++++----
> src/xen/xen_hypervisor.c | 11 ++++++-----
> src/xen/xen_hypervisor.h | 9 +++++----
> src/xen/xend_internal.c | 21 +++++++++++----------
> src/xen/xend_internal.h | 7 ++++++-
> src/xen/xm_internal.c | 3 ++-
> src/xen/xm_internal.h | 7 ++++++-
> 9 files changed, 73 insertions(+), 30 deletions(-)
>
> diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c
> index ded8d7f..56a3901 100644
> --- a/src/xen/block_stats.c
> +++ b/src/xen/block_stats.c
> @@ -359,16 +359,16 @@ xenLinuxDomainDeviceID(int domid, const char *path)
>
> int
> xenLinuxDomainBlockStats(xenUnifiedPrivatePtr priv,
> - virDomainPtr dom,
> + virDomainDefPtr def,
> const char *path,
> struct _virDomainBlockStats *stats)
> {
> - int device = xenLinuxDomainDeviceID(dom->id, path);
> + int device = xenLinuxDomainDeviceID(def->id, path);
>
> if (device < 0)
> return -1;
>
> - return read_bd_stats(priv, device, dom->id, stats);
> + return read_bd_stats(priv, device, def->id, stats);
> }
>
> #endif /* __linux__ */
> diff --git a/src/xen/block_stats.h b/src/xen/block_stats.h
> index 0a3c40a..6633d97 100644
> --- a/src/xen/block_stats.h
> +++ b/src/xen/block_stats.h
> @@ -28,7 +28,7 @@
> # include "xen_driver.h"
>
> extern int xenLinuxDomainBlockStats (xenUnifiedPrivatePtr priv,
> - virDomainPtr dom, const char *path,
> + virDomainDefPtr def, const char *path,
> struct _virDomainBlockStats *stats);
>
> extern int xenLinuxDomainDeviceID(int domid, const char *dev);
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 5ab1a52..7c00b70 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -1957,14 +1957,34 @@ static int
> xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path,
> struct _virDomainBlockStats *stats)
> {
> - return xenHypervisorDomainBlockStats(dom, path, stats);
> + virDomainDefPtr def = NULL;
> + int ret = -1;
> +
> + if (!(def = xenGetDomainDefForDom(dom)))
> + goto cleanup;
> +
> + ret = xenHypervisorDomainBlockStats(dom->conn, def, path, stats);
> +
> +cleanup:
> + virDomainDefFree(def);
> + return ret;
> }
>
> static int
> xenUnifiedDomainInterfaceStats(virDomainPtr dom, const char *path,
> struct _virDomainInterfaceStats *stats)
> {
> - return xenHypervisorDomainInterfaceStats(dom, path, stats);
> + virDomainDefPtr def = NULL;
> + int ret = -1;
> +
> + if (!(def = xenGetDomainDefForDom(dom)))
> + goto cleanup;
> +
> + ret = xenHypervisorDomainInterfaceStats(def, path, stats);
> +
> +cleanup:
> + virDomainDefFree(def);
> + return ret;
> }
>
> static int
> @@ -1973,13 +1993,22 @@ xenUnifiedDomainBlockPeek(virDomainPtr dom, const char *path,
> void *buffer, unsigned int flags)
> {
> xenUnifiedPrivatePtr priv = dom->conn->privateData;
> + virDomainDefPtr def = NULL;
> + int ret = -1;
>
> virCheckFlags(0, -1);
>
> + if (!(def = xenGetDomainDefForDom(dom)))
> + goto cleanup;
> +
> if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> - return xenXMDomainBlockPeek(dom, path, offset, size, buffer);
> + ret = xenXMDomainBlockPeek(dom->conn, def, path, offset, size, buffer);
> else
> - return xenDaemonDomainBlockPeek(dom, path, offset, size, buffer);
> + ret = xenDaemonDomainBlockPeek(dom->conn, def, path, offset, size, buffer);
> +
> +cleanup:
> + virDomainDefFree(def);
> + return ret;
> }
>
> static int
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 2525566..612ac77 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -1368,17 +1368,18 @@ xenHypervisorSetSchedulerParameters(virConnectPtr conn,
>
>
> int
> -xenHypervisorDomainBlockStats(virDomainPtr dom,
> +xenHypervisorDomainBlockStats(virConnectPtr conn,
> + virDomainDefPtr def,
> const char *path,
> struct _virDomainBlockStats *stats)
> {
> #ifdef __linux__
> - xenUnifiedPrivatePtr priv = dom->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
> int ret;
>
> xenUnifiedLock(priv);
> /* Need to lock because it hits the xenstore handle :-( */
> - ret = xenLinuxDomainBlockStats(priv, dom, path, stats);
> + ret = xenLinuxDomainBlockStats(priv, def, path, stats);
> xenUnifiedUnlock(priv);
> return ret;
> #else
> @@ -1396,7 +1397,7 @@ xenHypervisorDomainBlockStats(virDomainPtr dom,
> * virNetwork interface, as yet not decided.
> */
> int
> -xenHypervisorDomainInterfaceStats(virDomainPtr dom,
> +xenHypervisorDomainInterfaceStats(virDomainDefPtr def,
> const char *path,
> struct _virDomainInterfaceStats *stats)
> {
> @@ -1411,7 +1412,7 @@ xenHypervisorDomainInterfaceStats(virDomainPtr dom,
> _("invalid path, should be vif<domid>.<n>."));
> return -1;
> }
> - if (rqdomid != dom->id) {
> + if (rqdomid != def->id) {
> virReportError(VIR_ERR_INVALID_ARG, "%s",
> _("invalid path, vif<domid> should match this domain ID"));
> return -1;
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index 1e5bb67..6aeab79 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -122,13 +122,14 @@ int xenHypervisorSetSchedulerParameters(virConnectPtr conn,
> int nparams)
> ATTRIBUTE_NONNULL (1);
>
> -int xenHypervisorDomainBlockStats (virDomainPtr domain,
> +int xenHypervisorDomainBlockStats (virConnectPtr conn,
> + virDomainDefPtr def,
> const char *path,
> struct _virDomainBlockStats *stats)
> ATTRIBUTE_NONNULL (1);
> -int xenHypervisorDomainInterfaceStats (virDomainPtr domain,
> - const char *path,
> - struct _virDomainInterfaceStats *stats)
> +int xenHypervisorDomainInterfaceStats (virDomainDefPtr def,
> + const char *path,
> + struct _virDomainInterfaceStats *stats)
> ATTRIBUTE_NONNULL (1);
>
> int xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn,
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 3bcd19b..273408d 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -3247,13 +3247,14 @@ error:
> * Returns 0 if successful, -1 if error
> */
> int
> -xenDaemonDomainBlockPeek(virDomainPtr domain,
> +xenDaemonDomainBlockPeek(virConnectPtr conn,
> + virDomainDefPtr minidef,
>
Function comments need updated.
ACK.
Regards,
Jim
> const char *path,
> unsigned long long offset,
> size_t size,
> void *buffer)
> {
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
> struct sexpr *root = NULL;
> int fd = -1, ret = -1;
> virDomainDefPtr def;
> @@ -3263,12 +3264,12 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
> const char *actual;
>
> /* Security check: The path must correspond to a block device. */
> - if (domain->id > 0)
> - root = sexpr_get(domain->conn, "/xend/domain/%d?detail=1",
> - domain->id);
> - else if (domain->id < 0)
> - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1",
> - domain->name);
> + if (minidef->id > 0)
> + root = sexpr_get(conn, "/xend/domain/%d?detail=1",
> + minidef->id);
> + else if (minidef->id < 0)
> + root = sexpr_get(conn, "/xend/domain/%s?detail=1",
> + minidef->name);
> else {
> /* This call always fails for dom0. */
> virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -3283,8 +3284,8 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
>
> id = xenGetDomIdFromSxpr(root, priv->xendConfigVersion);
> xenUnifiedLock(priv);
> - tty = xenStoreDomainGetConsolePath(domain->conn, id);
> - vncport = xenStoreDomainGetVNCPort(domain->conn, id);
> + tty = xenStoreDomainGetConsolePath(conn, id);
> + vncport = xenStoreDomainGetVNCPort(conn, id);
> xenUnifiedUnlock(priv);
>
> if (!(def = xenParseSxpr(root, priv->xendConfigVersion, NULL, tty,
> diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
> index cef7da4..aa05130 100644
> --- a/src/xen/xend_internal.h
> +++ b/src/xen/xend_internal.h
> @@ -187,7 +187,12 @@ int xenDaemonDomainMigratePerform (virConnectPtr conn,
> const char *uri, unsigned long flags,
> const char *dname, unsigned long resource);
>
> -int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer);
> +int xenDaemonDomainBlockPeek(virConnectPtr conn,
> + virDomainDefPtr def,
> + const char *path,
> + unsigned long long offset,
> + size_t size,
> + void *buffer);
>
> char * xenDaemonGetSchedulerType(virConnectPtr conn,
> int *nparams);
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index bc98cf1..740c4df 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -1404,7 +1404,8 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn,
> }
>
> int
> -xenXMDomainBlockPeek(virDomainPtr dom ATTRIBUTE_UNUSED,
> +xenXMDomainBlockPeek(virConnectPtr conn ATTRIBUTE_UNUSED,
> + virDomainDefPtr def ATTRIBUTE_UNUSED,
> const char *path ATTRIBUTE_UNUSED,
> unsigned long long offset ATTRIBUTE_UNUSED,
> size_t size ATTRIBUTE_UNUSED,
> diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
> index 78cd15c..25b4fd5 100644
> --- a/src/xen/xm_internal.h
> +++ b/src/xen/xm_internal.h
> @@ -80,7 +80,12 @@ int xenXMDomainCreate(virConnectPtr conn,
> int xenXMDomainDefineXML(virConnectPtr con, virDomainDefPtr def);
> int xenXMDomainUndefine(virConnectPtr conn, virDomainDefPtr def);
>
> -int xenXMDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, size_t size, void *buffer);
> +int xenXMDomainBlockPeek(virConnectPtr conn,
> + virDomainDefPtr def,
> + const char *path,
> + unsigned long long offset,
> + size_t size,
> + void *buffer);
>
> int xenXMDomainGetAutostart(virDomainDefPtr def,
> int *autostart);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list