1. Patchew
  2. Libvirt
  3. View series

[libvirt PATCH] src: reject empty string for 'dname' in migrate APIs

Daniel P. Berrangé posted 1 patch 5 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20231108162357.1792115-1-berrange@redhat.com
src/internal.h       | 14 +++++++
src/libvirt-domain.c | 97 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 100 insertions(+), 11 deletions(-)
[libvirt PATCH] src: reject empty string for 'dname' in migrate APIs
Posted by Daniel P. Berrangé 5 months, 3 weeks ago 
A domain name is expected to be non-empty, and we validate this when
parsing XML, or accepting a new name during renames. We fail to
enforce this property, however, when performing a migration. This
was discovered when a user complained about inaccessible VMs after
migrating with the Rust APIs which mistakenly hardcoded 'dname' to
the empty string.

Fixes: https://gitlab.com/libvirt/libvirt-rust/-/issues/11
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/internal.h       | 14 +++++++
 src/libvirt-domain.c | 97 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 100 insertions(+), 11 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index 5a9e1c7cd0..01860efad9 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -441,6 +441,20 @@
             goto label; \
         } \
     } while (0)
+#define virCheckNonEmptyOptStringArgGoto(argname, label) \
+    do { \
+        if (argname && *argname == '\0') { \
+            virReportInvalidEmptyStringArg(argname); \
+            goto label; \
+        } \
+    } while (0)
+#define virCheckNonEmptyOptStringArgReturn(argname, retval) \
+    do { \
+        if (argname && *argname == '\0') { \
+            virReportInvalidEmptyStringArg(argname); \
+            return retval; \
+        } \
+    } while (0)
 #define virCheckPositiveArgGoto(argname, label) \
     do { \
         if (argname <= 0) { \
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 58e1e5ea8d..77a9682ecb 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -2991,6 +2991,8 @@ virDomainMigrateVersion1(virDomainPtr domain,
                      "dconn=%p, flags=0x%lx, dname=%s, uri=%s, bandwidth=%lu",
                      dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth);
 
+    virCheckNonEmptyOptStringArgReturn(dname, NULL);
+
     ret = virDomainGetInfo(domain, &info);
     if (ret == 0 && info.state == VIR_DOMAIN_PAUSED)
         flags |= VIR_MIGRATE_PAUSED;
@@ -3085,6 +3087,8 @@ virDomainMigrateVersion2(virDomainPtr domain,
                      "dconn=%p, flags=0x%lx, dname=%s, uri=%s, bandwidth=%lu",
                      dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth);
 
+    virCheckNonEmptyOptStringArgReturn(dname, NULL);
+
     /* Prepare the migration.
      *
      * The destination host may return a cookie, or leave cookie as
@@ -3242,6 +3246,8 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
                      bandwidth, params, nparams, useParams, flags);
     VIR_TYPED_PARAMS_DEBUG(params, nparams);
 
+    virCheckNonEmptyOptStringArgReturn(dname, NULL);
+
     if ((!useParams &&
          (!domain->conn->driver->domainMigrateBegin3 ||
           !domain->conn->driver->domainMigratePerform3 ||
@@ -3582,6 +3588,8 @@ virDomainMigrateUnmanagedProto2(virDomainPtr domain,
         return -1;
     }
 
+    virCheckNonEmptyOptStringArgReturn(dname, -1);
+
     if (flags & VIR_MIGRATE_PEER2PEER) {
         if (miguri) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -3632,6 +3640,8 @@ virDomainMigrateUnmanagedProto3(virDomainPtr domain,
         return -1;
     }
 
+    virCheckNonEmptyOptStringArgReturn(dname, -1);
+
     return domain->conn->driver->domainMigratePerform3
             (domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
              miguri, flags, dname, bandwidth);
@@ -3802,6 +3812,8 @@ virDomainMigrate(virDomainPtr domain,
     virCheckConnectGoto(dconn, error);
     virCheckReadOnlyGoto(dconn->flags, error);
 
+    virCheckNonEmptyOptStringArgReturn(dname, NULL);
+
     VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
                              VIR_MIGRATE_NON_SHARED_INC,
                              error);
@@ -3999,6 +4011,8 @@ virDomainMigrate2(virDomainPtr domain,
     virCheckConnectGoto(dconn, error);
     virCheckReadOnlyGoto(dconn->flags, error);
 
+    virCheckNonEmptyOptStringArgReturn(dname, NULL);
+
     VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
                              VIR_MIGRATE_NON_SHARED_INC,
                              error);
@@ -4232,6 +4246,19 @@ virDomainMigrate3(virDomainPtr domain,
         goto error;
     }
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_URI, &uri) < 0 ||
+        virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
+        virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_XML, &dxml) < 0 ||
+        virTypedParamsGetULLong(params, nparams,
+                                VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
+        goto error;
+    }
+
+    virCheckNonEmptyOptStringArgReturn(dname, NULL);
+
     if (flags & VIR_MIGRATE_OFFLINE) {
         rc = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
                                       VIR_DRV_FEATURE_MIGRATION_OFFLINE);
@@ -4293,17 +4320,6 @@ virDomainMigrate3(virDomainPtr domain,
         goto error;
     }
 
-    if (virTypedParamsGetString(params, nparams,
-                                VIR_MIGRATE_PARAM_URI, &uri) < 0 ||
-        virTypedParamsGetString(params, nparams,
-                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
-        virTypedParamsGetString(params, nparams,
-                                VIR_MIGRATE_PARAM_DEST_XML, &dxml) < 0 ||
-        virTypedParamsGetULLong(params, nparams,
-                                VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
-        goto error;
-    }
-
     rc_src = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
                                       VIR_DRV_FEATURE_MIGRATION_V3);
     if (rc_src < 0)
@@ -4475,6 +4491,7 @@ virDomainMigrateToURI(virDomainPtr domain,
     virCheckDomainReturn(domain, -1);
     virCheckReadOnlyGoto(domain->conn->flags, error);
     virCheckNonNullArgGoto(duri, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,
                              VIR_MIGRATE_PARALLEL,
@@ -4554,6 +4571,8 @@ virDomainMigrateToURI2(virDomainPtr domain,
     virCheckDomainReturn(domain, -1);
     virCheckReadOnlyGoto(domain->conn->flags, error);
 
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,
                              VIR_MIGRATE_PARALLEL,
                              error);
@@ -4679,6 +4698,7 @@ virDomainMigratePrepare(virConnectPtr dconn,
 
     virCheckConnectReturn(dconn, -1);
     virCheckReadOnlyGoto(dconn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (dconn->driver->domainMigratePrepare) {
         int ret;
@@ -4723,6 +4743,7 @@ virDomainMigratePerform(virDomainPtr domain,
     conn = domain->conn;
 
     virCheckReadOnlyGoto(conn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (conn->driver->domainMigratePerform) {
         int ret;
@@ -4762,6 +4783,7 @@ virDomainMigrateFinish(virConnectPtr dconn,
 
     virCheckConnectReturn(dconn, NULL);
     virCheckReadOnlyGoto(dconn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (dconn->driver->domainMigrateFinish) {
         virDomainPtr ret;
@@ -4805,6 +4827,7 @@ virDomainMigratePrepare2(virConnectPtr dconn,
 
     virCheckConnectReturn(dconn, -1);
     virCheckReadOnlyGoto(dconn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (dconn->driver->domainMigratePrepare2) {
         int ret;
@@ -4846,6 +4869,7 @@ virDomainMigrateFinish2(virConnectPtr dconn,
 
     virCheckConnectReturn(dconn, NULL);
     virCheckReadOnlyGoto(dconn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (dconn->driver->domainMigrateFinish2) {
         virDomainPtr ret;
@@ -4886,6 +4910,7 @@ virDomainMigratePrepareTunnel(virConnectPtr conn,
 
     virCheckConnectReturn(conn, -1);
     virCheckReadOnlyGoto(conn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (conn != st->conn) {
         virReportInvalidArg(conn, "%s",
@@ -4936,6 +4961,7 @@ virDomainMigrateBegin3(virDomainPtr domain,
     conn = domain->conn;
 
     virCheckReadOnlyGoto(conn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (conn->driver->domainMigrateBegin3) {
         char *xml;
@@ -4983,6 +5009,7 @@ virDomainMigratePrepare3(virConnectPtr dconn,
 
     virCheckConnectReturn(dconn, -1);
     virCheckReadOnlyGoto(dconn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (dconn->driver->domainMigratePrepare3) {
         int ret;
@@ -5031,6 +5058,7 @@ virDomainMigratePrepareTunnel3(virConnectPtr conn,
 
     virCheckConnectReturn(conn, -1);
     virCheckReadOnlyGoto(conn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (conn != st->conn) {
         virReportInvalidArg(conn, "%s",
@@ -5089,6 +5117,7 @@ virDomainMigratePerform3(virDomainPtr domain,
     conn = domain->conn;
 
     virCheckReadOnlyGoto(conn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (conn->driver->domainMigratePerform3) {
         int ret;
@@ -5135,6 +5164,7 @@ virDomainMigrateFinish3(virConnectPtr dconn,
 
     virCheckConnectReturn(dconn, NULL);
     virCheckReadOnlyGoto(dconn->flags, error);
+    virCheckNonEmptyOptStringArgGoto(dname, error);
 
     if (dconn->driver->domainMigrateFinish3) {
         virDomainPtr ret;
@@ -5211,6 +5241,7 @@ virDomainMigrateBegin3Params(virDomainPtr domain,
                              unsigned int flags)
 {
     virConnectPtr conn;
+    const char *dname = NULL;
 
     VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, "
                      "cookieout=%p, cookieoutlen=%p, flags=0x%x",
@@ -5224,6 +5255,12 @@ virDomainMigrateBegin3Params(virDomainPtr domain,
 
     virCheckReadOnlyGoto(conn->flags, error);
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto error;
+
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     if (conn->driver->domainMigrateBegin3Params) {
         char *xml;
         xml = conn->driver->domainMigrateBegin3Params(domain, params, nparams,
@@ -5258,6 +5295,8 @@ virDomainMigratePrepare3Params(virConnectPtr dconn,
                                char **uri_out,
                                unsigned int flags)
 {
+    const char *dname = NULL;
+
     VIR_DEBUG("dconn=%p, params=%p, nparams=%d, cookiein=%p, cookieinlen=%d, "
               "cookieout=%p, cookieoutlen=%p, uri_out=%p, flags=0x%x",
               dconn, params, nparams, cookiein, cookieinlen,
@@ -5269,6 +5308,12 @@ virDomainMigratePrepare3Params(virConnectPtr dconn,
     virCheckConnectReturn(dconn, -1);
     virCheckReadOnlyGoto(dconn->flags, error);
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto error;
+
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     if (dconn->driver->domainMigratePrepare3Params) {
         int ret;
         ret = dconn->driver->domainMigratePrepare3Params(dconn, params, nparams,
@@ -5303,6 +5348,8 @@ virDomainMigratePrepareTunnel3Params(virConnectPtr conn,
                                      int *cookieoutlen,
                                      unsigned int flags)
 {
+    const char *dname = NULL;
+
     VIR_DEBUG("conn=%p, stream=%p, params=%p, nparams=%d, cookiein=%p, "
               "cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=0x%x",
               conn, st, params, nparams, cookiein, cookieinlen,
@@ -5320,6 +5367,12 @@ virDomainMigratePrepareTunnel3Params(virConnectPtr conn,
         goto error;
     }
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto error;
+
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     if (conn->driver->domainMigratePrepareTunnel3Params) {
         int rv;
         rv = conn->driver->domainMigratePrepareTunnel3Params(
@@ -5354,6 +5407,7 @@ virDomainMigratePerform3Params(virDomainPtr domain,
                                unsigned int flags)
 {
     virConnectPtr conn;
+    const char *dname = NULL;
 
     VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, cookiein=%p, "
                      "cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=0x%x",
@@ -5368,6 +5422,12 @@ virDomainMigratePerform3Params(virDomainPtr domain,
 
     virCheckReadOnlyGoto(conn->flags, error);
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto error;
+
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     if (conn->driver->domainMigratePerform3Params) {
         int ret;
         ret = conn->driver->domainMigratePerform3Params(
@@ -5401,6 +5461,8 @@ virDomainMigrateFinish3Params(virConnectPtr dconn,
                               unsigned int flags,
                               int cancelled)
 {
+    const char *dname = NULL;
+
     VIR_DEBUG("dconn=%p, params=%p, nparams=%d, cookiein=%p, cookieinlen=%d, "
               "cookieout=%p, cookieoutlen=%p, flags=0x%x, cancelled=%d",
               dconn, params, nparams, cookiein, cookieinlen, cookieout,
@@ -5412,6 +5474,12 @@ virDomainMigrateFinish3Params(virConnectPtr dconn,
     virCheckConnectReturn(dconn, NULL);
     virCheckReadOnlyGoto(dconn->flags, error);
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto error;
+
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     if (dconn->driver->domainMigrateFinish3Params) {
         virDomainPtr ret;
         ret = dconn->driver->domainMigrateFinish3Params(
@@ -5444,6 +5512,7 @@ virDomainMigrateConfirm3Params(virDomainPtr domain,
                                int cancelled)
 {
     virConnectPtr conn;
+    const char *dname = NULL;
 
     VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, cookiein=%p, "
                      "cookieinlen=%d, flags=0x%x, cancelled=%d",
@@ -5457,6 +5526,12 @@ virDomainMigrateConfirm3Params(virDomainPtr domain,
 
     virCheckReadOnlyGoto(conn->flags, error);
 
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto error;
+
+    virCheckNonEmptyOptStringArgGoto(dname, error);
+
     if (conn->driver->domainMigrateConfirm3Params) {
         int ret;
         ret = conn->driver->domainMigrateConfirm3Params(
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] src: reject empty string for 'dname' in migrate APIs
Posted by Jiri Denemark 5 months, 2 weeks ago 
On Wed, Nov 08, 2023 at 16:23:57 +0000, Daniel P. Berrangé wrote:
> A domain name is expected to be non-empty, and we validate this when
> parsing XML, or accepting a new name during renames. We fail to
> enforce this property, however, when performing a migration. This
> was discovered when a user complained about inaccessible VMs after
> migrating with the Rust APIs which mistakenly hardcoded 'dname' to
> the empty string.
> 
> Fixes: https://gitlab.com/libvirt/libvirt-rust/-/issues/11
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/internal.h       | 14 +++++++
>  src/libvirt-domain.c | 97 +++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 100 insertions(+), 11 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.