src: reject empty string for 'dname' in migrate APIs

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
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2023-11-08 16:20:35 +00:00
parent 66e84b8a95
commit a585ef905a
2 changed files with 100 additions and 11 deletions

View File

@ -441,6 +441,20 @@
goto label; \ goto label; \
} \ } \
} while (0) } 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) \ #define virCheckPositiveArgGoto(argname, label) \
do { \ do { \
if (argname <= 0) { \ if (argname <= 0) { \

View File

@ -2991,6 +2991,8 @@ virDomainMigrateVersion1(virDomainPtr domain,
"dconn=%p, flags=0x%lx, dname=%s, uri=%s, bandwidth=%lu", "dconn=%p, flags=0x%lx, dname=%s, uri=%s, bandwidth=%lu",
dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth);
virCheckNonEmptyOptStringArgReturn(dname, NULL);
ret = virDomainGetInfo(domain, &info); ret = virDomainGetInfo(domain, &info);
if (ret == 0 && info.state == VIR_DOMAIN_PAUSED) if (ret == 0 && info.state == VIR_DOMAIN_PAUSED)
flags |= VIR_MIGRATE_PAUSED; flags |= VIR_MIGRATE_PAUSED;
@ -3085,6 +3087,8 @@ virDomainMigrateVersion2(virDomainPtr domain,
"dconn=%p, flags=0x%lx, dname=%s, uri=%s, bandwidth=%lu", "dconn=%p, flags=0x%lx, dname=%s, uri=%s, bandwidth=%lu",
dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth);
virCheckNonEmptyOptStringArgReturn(dname, NULL);
/* Prepare the migration. /* Prepare the migration.
* *
* The destination host may return a cookie, or leave cookie as * The destination host may return a cookie, or leave cookie as
@ -3242,6 +3246,8 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
bandwidth, params, nparams, useParams, flags); bandwidth, params, nparams, useParams, flags);
VIR_TYPED_PARAMS_DEBUG(params, nparams); VIR_TYPED_PARAMS_DEBUG(params, nparams);
virCheckNonEmptyOptStringArgReturn(dname, NULL);
if ((!useParams && if ((!useParams &&
(!domain->conn->driver->domainMigrateBegin3 || (!domain->conn->driver->domainMigrateBegin3 ||
!domain->conn->driver->domainMigratePerform3 || !domain->conn->driver->domainMigratePerform3 ||
@ -3582,6 +3588,8 @@ virDomainMigrateUnmanagedProto2(virDomainPtr domain,
return -1; return -1;
} }
virCheckNonEmptyOptStringArgReturn(dname, -1);
if (flags & VIR_MIGRATE_PEER2PEER) { if (flags & VIR_MIGRATE_PEER2PEER) {
if (miguri) { if (miguri) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@ -3632,6 +3640,8 @@ virDomainMigrateUnmanagedProto3(virDomainPtr domain,
return -1; return -1;
} }
virCheckNonEmptyOptStringArgReturn(dname, -1);
return domain->conn->driver->domainMigratePerform3 return domain->conn->driver->domainMigratePerform3
(domain, xmlin, NULL, 0, NULL, NULL, dconnuri, (domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
miguri, flags, dname, bandwidth); miguri, flags, dname, bandwidth);
@ -3802,6 +3812,8 @@ virDomainMigrate(virDomainPtr domain,
virCheckConnectGoto(dconn, error); virCheckConnectGoto(dconn, error);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgReturn(dname, NULL);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK, VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
VIR_MIGRATE_NON_SHARED_INC, VIR_MIGRATE_NON_SHARED_INC,
error); error);
@ -3999,6 +4011,8 @@ virDomainMigrate2(virDomainPtr domain,
virCheckConnectGoto(dconn, error); virCheckConnectGoto(dconn, error);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgReturn(dname, NULL);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK, VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
VIR_MIGRATE_NON_SHARED_INC, VIR_MIGRATE_NON_SHARED_INC,
error); error);
@ -4232,6 +4246,19 @@ virDomainMigrate3(virDomainPtr domain,
goto error; 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) { if (flags & VIR_MIGRATE_OFFLINE) {
rc = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, rc = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE); VIR_DRV_FEATURE_MIGRATION_OFFLINE);
@ -4293,17 +4320,6 @@ virDomainMigrate3(virDomainPtr domain,
goto error; 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, rc_src = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_V3); VIR_DRV_FEATURE_MIGRATION_V3);
if (rc_src < 0) if (rc_src < 0)
@ -4475,6 +4491,7 @@ virDomainMigrateToURI(virDomainPtr domain,
virCheckDomainReturn(domain, -1); virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error); virCheckReadOnlyGoto(domain->conn->flags, error);
virCheckNonNullArgGoto(duri, error); virCheckNonNullArgGoto(duri, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED, VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,
VIR_MIGRATE_PARALLEL, VIR_MIGRATE_PARALLEL,
@ -4554,6 +4571,8 @@ virDomainMigrateToURI2(virDomainPtr domain,
virCheckDomainReturn(domain, -1); virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error); virCheckReadOnlyGoto(domain->conn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED, VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,
VIR_MIGRATE_PARALLEL, VIR_MIGRATE_PARALLEL,
error); error);
@ -4679,6 +4698,7 @@ virDomainMigratePrepare(virConnectPtr dconn,
virCheckConnectReturn(dconn, -1); virCheckConnectReturn(dconn, -1);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigratePrepare) { if (dconn->driver->domainMigratePrepare) {
int ret; int ret;
@ -4723,6 +4743,7 @@ virDomainMigratePerform(virDomainPtr domain,
conn = domain->conn; conn = domain->conn;
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigratePerform) { if (conn->driver->domainMigratePerform) {
int ret; int ret;
@ -4762,6 +4783,7 @@ virDomainMigrateFinish(virConnectPtr dconn,
virCheckConnectReturn(dconn, NULL); virCheckConnectReturn(dconn, NULL);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigrateFinish) { if (dconn->driver->domainMigrateFinish) {
virDomainPtr ret; virDomainPtr ret;
@ -4805,6 +4827,7 @@ virDomainMigratePrepare2(virConnectPtr dconn,
virCheckConnectReturn(dconn, -1); virCheckConnectReturn(dconn, -1);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigratePrepare2) { if (dconn->driver->domainMigratePrepare2) {
int ret; int ret;
@ -4846,6 +4869,7 @@ virDomainMigrateFinish2(virConnectPtr dconn,
virCheckConnectReturn(dconn, NULL); virCheckConnectReturn(dconn, NULL);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigrateFinish2) { if (dconn->driver->domainMigrateFinish2) {
virDomainPtr ret; virDomainPtr ret;
@ -4886,6 +4910,7 @@ virDomainMigratePrepareTunnel(virConnectPtr conn,
virCheckConnectReturn(conn, -1); virCheckConnectReturn(conn, -1);
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn != st->conn) { if (conn != st->conn) {
virReportInvalidArg(conn, "%s", virReportInvalidArg(conn, "%s",
@ -4936,6 +4961,7 @@ virDomainMigrateBegin3(virDomainPtr domain,
conn = domain->conn; conn = domain->conn;
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigrateBegin3) { if (conn->driver->domainMigrateBegin3) {
char *xml; char *xml;
@ -4983,6 +5009,7 @@ virDomainMigratePrepare3(virConnectPtr dconn,
virCheckConnectReturn(dconn, -1); virCheckConnectReturn(dconn, -1);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigratePrepare3) { if (dconn->driver->domainMigratePrepare3) {
int ret; int ret;
@ -5031,6 +5058,7 @@ virDomainMigratePrepareTunnel3(virConnectPtr conn,
virCheckConnectReturn(conn, -1); virCheckConnectReturn(conn, -1);
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn != st->conn) { if (conn != st->conn) {
virReportInvalidArg(conn, "%s", virReportInvalidArg(conn, "%s",
@ -5089,6 +5117,7 @@ virDomainMigratePerform3(virDomainPtr domain,
conn = domain->conn; conn = domain->conn;
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigratePerform3) { if (conn->driver->domainMigratePerform3) {
int ret; int ret;
@ -5135,6 +5164,7 @@ virDomainMigrateFinish3(virConnectPtr dconn,
virCheckConnectReturn(dconn, NULL); virCheckConnectReturn(dconn, NULL);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigrateFinish3) { if (dconn->driver->domainMigrateFinish3) {
virDomainPtr ret; virDomainPtr ret;
@ -5211,6 +5241,7 @@ virDomainMigrateBegin3Params(virDomainPtr domain,
unsigned int flags) unsigned int flags)
{ {
virConnectPtr conn; virConnectPtr conn;
const char *dname = NULL;
VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, " VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, "
"cookieout=%p, cookieoutlen=%p, flags=0x%x", "cookieout=%p, cookieoutlen=%p, flags=0x%x",
@ -5224,6 +5255,12 @@ virDomainMigrateBegin3Params(virDomainPtr domain,
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
goto error;
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigrateBegin3Params) { if (conn->driver->domainMigrateBegin3Params) {
char *xml; char *xml;
xml = conn->driver->domainMigrateBegin3Params(domain, params, nparams, xml = conn->driver->domainMigrateBegin3Params(domain, params, nparams,
@ -5258,6 +5295,8 @@ virDomainMigratePrepare3Params(virConnectPtr dconn,
char **uri_out, char **uri_out,
unsigned int flags) unsigned int flags)
{ {
const char *dname = NULL;
VIR_DEBUG("dconn=%p, params=%p, nparams=%d, cookiein=%p, cookieinlen=%d, " VIR_DEBUG("dconn=%p, params=%p, nparams=%d, cookiein=%p, cookieinlen=%d, "
"cookieout=%p, cookieoutlen=%p, uri_out=%p, flags=0x%x", "cookieout=%p, cookieoutlen=%p, uri_out=%p, flags=0x%x",
dconn, params, nparams, cookiein, cookieinlen, dconn, params, nparams, cookiein, cookieinlen,
@ -5269,6 +5308,12 @@ virDomainMigratePrepare3Params(virConnectPtr dconn,
virCheckConnectReturn(dconn, -1); virCheckConnectReturn(dconn, -1);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
goto error;
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigratePrepare3Params) { if (dconn->driver->domainMigratePrepare3Params) {
int ret; int ret;
ret = dconn->driver->domainMigratePrepare3Params(dconn, params, nparams, ret = dconn->driver->domainMigratePrepare3Params(dconn, params, nparams,
@ -5303,6 +5348,8 @@ virDomainMigratePrepareTunnel3Params(virConnectPtr conn,
int *cookieoutlen, int *cookieoutlen,
unsigned int flags) unsigned int flags)
{ {
const char *dname = NULL;
VIR_DEBUG("conn=%p, stream=%p, params=%p, nparams=%d, cookiein=%p, " VIR_DEBUG("conn=%p, stream=%p, params=%p, nparams=%d, cookiein=%p, "
"cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=0x%x", "cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=0x%x",
conn, st, params, nparams, cookiein, cookieinlen, conn, st, params, nparams, cookiein, cookieinlen,
@ -5320,6 +5367,12 @@ virDomainMigratePrepareTunnel3Params(virConnectPtr conn,
goto error; goto error;
} }
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
goto error;
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigratePrepareTunnel3Params) { if (conn->driver->domainMigratePrepareTunnel3Params) {
int rv; int rv;
rv = conn->driver->domainMigratePrepareTunnel3Params( rv = conn->driver->domainMigratePrepareTunnel3Params(
@ -5354,6 +5407,7 @@ virDomainMigratePerform3Params(virDomainPtr domain,
unsigned int flags) unsigned int flags)
{ {
virConnectPtr conn; virConnectPtr conn;
const char *dname = NULL;
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, cookiein=%p, " VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, cookiein=%p, "
"cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=0x%x", "cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=0x%x",
@ -5368,6 +5422,12 @@ virDomainMigratePerform3Params(virDomainPtr domain,
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
goto error;
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigratePerform3Params) { if (conn->driver->domainMigratePerform3Params) {
int ret; int ret;
ret = conn->driver->domainMigratePerform3Params( ret = conn->driver->domainMigratePerform3Params(
@ -5401,6 +5461,8 @@ virDomainMigrateFinish3Params(virConnectPtr dconn,
unsigned int flags, unsigned int flags,
int cancelled) int cancelled)
{ {
const char *dname = NULL;
VIR_DEBUG("dconn=%p, params=%p, nparams=%d, cookiein=%p, cookieinlen=%d, " VIR_DEBUG("dconn=%p, params=%p, nparams=%d, cookiein=%p, cookieinlen=%d, "
"cookieout=%p, cookieoutlen=%p, flags=0x%x, cancelled=%d", "cookieout=%p, cookieoutlen=%p, flags=0x%x, cancelled=%d",
dconn, params, nparams, cookiein, cookieinlen, cookieout, dconn, params, nparams, cookiein, cookieinlen, cookieout,
@ -5412,6 +5474,12 @@ virDomainMigrateFinish3Params(virConnectPtr dconn,
virCheckConnectReturn(dconn, NULL); virCheckConnectReturn(dconn, NULL);
virCheckReadOnlyGoto(dconn->flags, error); virCheckReadOnlyGoto(dconn->flags, error);
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
goto error;
virCheckNonEmptyOptStringArgGoto(dname, error);
if (dconn->driver->domainMigrateFinish3Params) { if (dconn->driver->domainMigrateFinish3Params) {
virDomainPtr ret; virDomainPtr ret;
ret = dconn->driver->domainMigrateFinish3Params( ret = dconn->driver->domainMigrateFinish3Params(
@ -5444,6 +5512,7 @@ virDomainMigrateConfirm3Params(virDomainPtr domain,
int cancelled) int cancelled)
{ {
virConnectPtr conn; virConnectPtr conn;
const char *dname = NULL;
VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, cookiein=%p, " VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, cookiein=%p, "
"cookieinlen=%d, flags=0x%x, cancelled=%d", "cookieinlen=%d, flags=0x%x, cancelled=%d",
@ -5457,6 +5526,12 @@ virDomainMigrateConfirm3Params(virDomainPtr domain,
virCheckReadOnlyGoto(conn->flags, error); virCheckReadOnlyGoto(conn->flags, error);
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
goto error;
virCheckNonEmptyOptStringArgGoto(dname, error);
if (conn->driver->domainMigrateConfirm3Params) { if (conn->driver->domainMigrateConfirm3Params) {
int ret; int ret;
ret = conn->driver->domainMigrateConfirm3Params( ret = conn->driver->domainMigrateConfirm3Params(