Re: [libvirt] [PATCH] Add missing OOM error checks, reports and cleanups

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index c2c5a44..c5083cc 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1057,13 +1057,18 @@ virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt, int create /* Extract device name */ if (create == EXISTING_DEVICE) { def->name = virXPathString(conn, "string(./name[1])", ctxt); + + if (!def->name) { + virNodeDeviceReportError(conn, VIR_ERR_NO_NAME, NULL); + goto error; + } } else { def->name = strdup("new device"); - } - if (!def->name) { - virNodeDeviceReportError(conn, VIR_ERR_NO_NAME, NULL); - goto error; + if (!def->name) { + virReportOOMError(conn); + goto error; + } } /* Extract device parent, if any */

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ee7a046..c866111 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -870,12 +870,12 @@ doRemoteOpen (virConnectPtr conn, } if(VIR_ALLOC(priv->callbackList)<0) { - error(conn, VIR_ERR_INVALID_ARG, _("Error allocating callbacks list")); + virReportOOMError(conn); goto failed; } if(VIR_ALLOC(priv->domainEvents)<0) { - error(conn, VIR_ERR_INVALID_ARG, _("Error allocating domainEvents")); + virReportOOMError(conn); goto failed; } @@ -2751,9 +2751,18 @@ remoteListDefinedDomains (virConnectPtr conn, char **const names, int maxnames) * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -3086,7 +3095,7 @@ remoteDomainSetSchedulerParameters (virDomainPtr domain, /* Serialise the scheduler parameters. */ args.params.params_len = nparams; if (VIR_ALLOC_N(args.params.params_val, nparams) < 0) { - error (domain->conn, VIR_ERR_RPC, _("out of memory allocating array")); + virReportOOMError(domain->conn); goto done; } @@ -3432,9 +3441,18 @@ remoteListNetworks (virConnectPtr conn, char **const names, int maxnames) * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -3505,9 +3523,18 @@ remoteListDefinedNetworks (virConnectPtr conn, * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -3921,9 +3948,18 @@ remoteListInterfaces (virConnectPtr conn, char **const names, int maxnames) * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -3993,9 +4029,18 @@ remoteListDefinedInterfaces (virConnectPtr conn, char **const names, int maxname * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -4314,9 +4359,18 @@ remoteListStoragePools (virConnectPtr conn, char **const names, int maxnames) * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -4383,9 +4437,18 @@ remoteListDefinedStoragePools (virConnectPtr conn, * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -4890,9 +4953,18 @@ remoteStoragePoolListVolumes (virStoragePoolPtr pool, char **const names, int ma * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(pool->conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -5290,9 +5362,18 @@ static int remoteNodeListDevices(virConnectPtr conn, * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -5443,9 +5524,18 @@ static int remoteNodeDeviceListCaps(virNodeDevicePtr dev, * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.names.names_len; ++i) + for (i = 0; i < ret.names.names_len; ++i) { names[i] = strdup (ret.names.names_val[i]); + if (names[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(names[i]); + + virReportOOMError(dev->conn); + goto cleanup; + } + } + rv = ret.names.names_len; cleanup: @@ -6496,9 +6586,18 @@ remoteSecretListSecrets (virConnectPtr conn, char **uuids, int maxuuids) * names and the list of pointers, so we have to strdup the * names here. */ - for (i = 0; i < ret.uuids.uuids_len; ++i) + for (i = 0; i < ret.uuids.uuids_len; ++i) { uuids[i] = strdup (ret.uuids.uuids_val[i]); + if (uuids[i] == NULL) { + for (--i; i >= 0; --i) + VIR_FREE(uuids[i]); + + virReportOOMError(conn); + goto cleanup; + } + } + rv = ret.uuids.uuids_len; cleanup: @@ -6707,8 +6806,10 @@ remoteStreamOpen(virStreamPtr st, struct private_data *priv = st->conn->privateData; struct private_stream_data *stpriv; - if (VIR_ALLOC(stpriv) < 0) + if (VIR_ALLOC(stpriv) < 0) { + virReportOOMError(st->conn); return NULL; + } /* Initialize call object used to receive replies */ stpriv->proc_nr = proc_nr; @@ -7106,8 +7207,10 @@ prepareCall(virConnectPtr conn, struct remote_message_header hdr; struct remote_thread_call *rv; - if (VIR_ALLOC(rv) < 0) + if (VIR_ALLOC(rv) < 0) { + virReportOOMError(conn); return NULL; + } if (virCondInit(&rv->cond) < 0) { VIR_FREE(rv); diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 1d5b4f7..1eef468 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -449,8 +449,10 @@ secretLoad(virConnectPtr conn, virSecretDriverStatePtr driver, if (secretLoadValidateUUID(conn, def, xml_basename) < 0) goto cleanup; - if (VIR_ALLOC(secret) < 0) + if (VIR_ALLOC(secret) < 0) { + virReportOOMError(conn); goto cleanup; + } secret->def = def; def = NULL; @@ -578,8 +580,10 @@ secretListSecrets(virConnectPtr conn, char **uuids, int maxuuids) char *uuidstr; if (i == maxuuids) break; - if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) + if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) { + virReportOOMError(conn); goto cleanup; + } virUUIDFormat(secret->def->uuid, uuidstr); uuids[i] = uuidstr; i++;

@@ -2748,6 +2759,7 @@ cleanup: struct xenXMListIteratorContext { virConnectPtr conn; + int oom; int max; int count; char ** names; @@ -2757,13 +2769,18 @@ static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const char *name, struct xenXMListIteratorContext *ctx = data; virDomainPtr dom = NULL; + if (ctx->oom) + return; + if (ctx->count == ctx->max) return; dom = xenDaemonLookupByName(ctx->conn, name); if (!dom) { - ctx->names[ctx->count] = strdup(name); - ctx->count++; + if (!(ctx->names[ctx->count] = strdup(name))) + ctx->oom = 1; + else + ctx->count++; } else { virDomainFree(dom); } @@ -2777,7 +2794,7 @@ static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const char *name, int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { xenUnifiedPrivatePtr priv; struct xenXMListIteratorContext ctx; - int ret = -1; + int i, ret = -1; if (!VIR_IS_CONNECT(conn)) { xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__); @@ -2794,11 +2811,21 @@ int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames maxnames = virHashSize(priv->configCache); ctx.conn = conn; + ctx.oom = 0; ctx.count = 0; ctx.max = maxnames; ctx.names = names; virHashForEach(priv->nameConfigMap, xenXMListIterator, &ctx); + + if (ctx.oom) { + for (i = 0; i < ctx.count; i++) + VIR_FREE(ctx.names[i]); + + virReportOOMError(conn); + goto cleanup; + } + ret = ctx.count; cleanup: