External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-23 06:05:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

conf: Utilize more of VIR_(APPEND|INSERT|DELETE)_ELEMENT

Browse Source 
This fixes a possible double free. In virNetworkAssignDef() if
virBitmapNew() fails, then virNetworkObjFree(network) is called.
However, with network->def pointing to actual @def. So if caller
frees @def again, ...

Moreover, this fixes one possible memory leak too. In
virInterfaceAssignDef() if appending to the list of interfaces
fails, we ought to call virInterfaceObjFree() instead of bare
VIR_FREE().

Although, in order to do that some array size variables needs
to be turned into size_t rather than int.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Michal Privoznik 2014-03-07 09:33:31 +01:00
parent 925de19ed7
commit 2133441a07
15 changed files with 74 additions and 214 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

120
src/conf/domain_conf.c
View File

@ -10216,17 +10216,9 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
}
}
/* No disks with this bus yet, so put at end of list */
if (insertAt == -1)
insertAt = def->ndisks;
if (insertAt < def->ndisks)
memmove(def->disks + insertAt + 1,
def->disks + insertAt,
(sizeof(def->disks[0]) * (def->ndisks-insertAt)));
def->disks[insertAt] = disk;
def->ndisks++;
/* VIR_INSERT_ELEMENT_INPLACE will never return an error here. */
ignore_value(VIR_INSERT_ELEMENT_INPLACE(def->disks, insertAt,
def->ndisks, disk));
}
@ -10235,19 +10227,7 @@ virDomainDiskRemove(virDomainDefPtr def, size_t i)
{
virDomainDiskDefPtr disk = def->disks[i];
if (def->ndisks > 1) {
memmove(def->disks + i,
def->disks + i + 1,
sizeof(*def->disks) *
(def->ndisks - (i + 1)));
def->ndisks--;
if (VIR_REALLOC_N(def->disks, def->ndisks) < 0) {
/* ignore, harmless */
}
} else {
VIR_FREE(def->disks);
def->ndisks = 0;
}
VIR_DELETE_ELEMENT(def->disks, i, def->ndisks);
return disk;
}
@ -10274,13 +10254,17 @@ virDomainHasDiskMirror(virDomainObjPtr vm)
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
{
if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0)
/* hostdev net devices must also exist in the hostdevs array */
if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV &&
virDomainHostdevInsert(def, &net->data.hostdev.def) < 0)
return -1;
if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) {
/* virDomainHostdevInsert just appends new hostdevs, so we are sure
* that the hostdev we've added a few lines above is at the end of
* array. Although, devices are indexed from zero ... */
virDomainHostdevRemove(def, def->nhostdevs - 1);
return -1;
def->nets[def->nnets] = net;
def->nnets++;
if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* hostdev net devices must also exist in the hostdevs array */
return virDomainHostdevInsert(def, &net->data.hostdev.def);
}
return 0;
}
@ -10360,19 +10344,7 @@ virDomainNetRemove(virDomainDefPtr def, size_t i)
virDomainNetDefPtr net = def->nets[i];
virDomainNetRemoveHostdev(def, net);
if (def->nnets > 1) {
memmove(def->nets + i,
def->nets + i + 1,
sizeof(*def->nets) * (def->nnets - (i + 1)));
def->nnets--;
if (VIR_REALLOC_N(def->nets, def->nnets) < 0) {
/* ignore harmless */
}
} else {
VIR_FREE(def->nets);
def->nnets = 0;
}
VIR_DELETE_ELEMENT(def->nets, i, def->nnets);
return net;
}
@ -10415,17 +10387,9 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def,
}
}
/* No controllers with this bus yet, so put at end of list */
if (insertAt == -1)
insertAt = def->ncontrollers;
if (insertAt < def->ncontrollers)
memmove(def->controllers + insertAt + 1,
def->controllers + insertAt,
(sizeof(def->controllers[0]) * (def->ncontrollers-insertAt)));
def->controllers[insertAt] = controller;
def->ncontrollers++;
/* VIR_INSERT_ELEMENT_INPLACE will never return an error here. */
ignore_value(VIR_INSERT_ELEMENT_INPLACE(def->controllers, insertAt,
def->ncontrollers, controller));
}
int
@ -10449,20 +10413,7 @@ virDomainControllerRemove(virDomainDefPtr def, size_t i)
{
virDomainControllerDefPtr controller = def->controllers[i];
if (def->ncontrollers > 1) {
memmove(def->controllers + i,
def->controllers + i + 1,
sizeof(*def->controllers) *
(def->ncontrollers - (i + 1)));
def->ncontrollers--;
if (VIR_REALLOC_N(def->controllers, def->ncontrollers) < 0) {
/* ignore, harmless */
}
} else {
VIR_FREE(def->controllers);
def->ncontrollers = 0;
}
VIR_DELETE_ELEMENT(def->controllers, i, def->ncontrollers);
return controller;
}
@ -10520,16 +10471,7 @@ virDomainLeaseRemoveAt(virDomainDefPtr def, size_t i)
virDomainLeaseDefPtr lease = def->leases[i];
if (def->nleases > 1) {
memmove(def->leases + i,
def->leases + i + 1,
sizeof(*def->leases) *
(def->nleases - (i + 1)));
VIR_SHRINK_N(def->leases, def->nleases, 1);
} else {
VIR_FREE(def->leases);
def->nleases = 0;
}
VIR_DELETE_ELEMENT(def->leases, i, def->nleases);
return lease;
}
@ -14451,7 +14393,7 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def,
int
virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list,
int *nvcpupin,
size_t *nvcpupin,
unsigned char *cpumap,
int maplen,
int vcpu)
@ -14484,11 +14426,9 @@ virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list,
if (!vcpupin->cpumask)
goto error;
if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) < 0)
if (VIR_APPEND_ELEMENT(*vcpupin_list, *nvcpupin, vcpupin) < 0)
goto error;
(*vcpupin_list)[(*nvcpupin)++] = vcpupin;
return 0;
error:
@ -14500,7 +14440,6 @@ int
virDomainVcpuPinDel(virDomainDefPtr def, int vcpu)
{
int n;
bool deleted = false;
virDomainVcpuPinDefPtr *vcpupin_list = def->cputune.vcpupin;
/* No vcpupin exists yet */
@ -14512,24 +14451,11 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu)
if (vcpupin_list[n]->vcpuid == vcpu) {
virBitmapFree(vcpupin_list[n]->cpumask);
VIR_FREE(vcpupin_list[n]);
memmove(&vcpupin_list[n],
&vcpupin_list[n+1],
(def->cputune.nvcpupin - n - 1) * sizeof(virDomainVcpuPinDef *));
deleted = true;
VIR_DELETE_ELEMENT(vcpupin_list, n, def->cputune.nvcpupin);
break;
}
}
if (!deleted)
return 0;
if (--def->cputune.nvcpupin == 0) {
VIR_FREE(def->cputune.vcpupin);
} else {
if (VIR_REALLOC_N(def->cputune.vcpupin, def->cputune.nvcpupin) < 0)
return -1;
}
return 0;
}

4
src/conf/domain_conf.h
View File

@ -2003,7 +2003,7 @@ struct _virDomainDef {
long long quota;
unsigned long long emulator_period;
long long emulator_quota;
int nvcpupin;
size_t nvcpupin;
virDomainVcpuPinDefPtr *vcpupin;
virDomainVcpuPinDefPtr emulatorpin;
} cputune;
@ -2410,7 +2410,7 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def,
virDomainDeviceDefPtr dev);
int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list,
int *nvcpupin,
size_t *nvcpupin,
unsigned char *cpumap,
int maplen,
int vcpu);

20
src/conf/interface_conf.c
View File

@ -1266,16 +1266,14 @@ virInterfaceObjPtr virInterfaceAssignDef(virInterfaceObjListPtr interfaces,
return NULL;
}
virInterfaceObjLock(iface);
iface->def = def;
if (VIR_REALLOC_N(interfaces->objs, interfaces->count + 1) < 0) {
VIR_FREE(iface);
if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
interfaces->count, iface) < 0) {
virInterfaceObjFree(iface);
return NULL;
}
interfaces->objs[interfaces->count] = iface;
interfaces->count++;
iface->def = def;
return iface;
}
@ -1292,15 +1290,7 @@ void virInterfaceRemove(virInterfaceObjListPtr interfaces,
virInterfaceObjUnlock(interfaces->objs[i]);
virInterfaceObjFree(interfaces->objs[i]);
if (i < (interfaces->count - 1))
memmove(interfaces->objs + i, interfaces->objs + i + 1,
sizeof(*(interfaces->objs)) * (interfaces->count - (i + 1)));
if (VIR_REALLOC_N(interfaces->objs, interfaces->count - 1) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
interfaces->count--;
VIR_DELETE_ELEMENT(interfaces->objs, i, interfaces->count);
break;
}
virInterfaceObjUnlock(interfaces->objs[i]);

2
src/conf/interface_conf.h
View File

@ -171,7 +171,7 @@ struct _virInterfaceObj {
typedef struct _virInterfaceObjList virInterfaceObjList;
typedef virInterfaceObjList *virInterfaceObjListPtr;
struct _virInterfaceObjList {
unsigned int count;
size_t count;
virInterfaceObjPtr *objs;
};

31
src/conf/network_conf.c
View File

@ -372,9 +372,6 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
return network;
}
if (VIR_REALLOC_N(nets->objs, nets->count + 1) < 0)
return NULL;
if (VIR_ALLOC(network) < 0)
return NULL;
if (virMutexInit(&network->lock) < 0) {
@ -384,9 +381,9 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
return NULL;
}
virNetworkObjLock(network);
network->def = def;
if (!(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0 ||
!(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
goto error;
/* The first three class IDs are already taken */
@ -395,8 +392,6 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
ignore_value(virBitmapSetBit(network->class_id, 2));
network->def = def;
nets->objs[nets->count] = network;
nets->count++;
return network;
error:
@ -576,15 +571,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
virNetworkObjUnlock(nets->objs[i]);
virNetworkObjFree(nets->objs[i]);
if (i < (nets->count - 1))
memmove(nets->objs + i, nets->objs + i + 1,
sizeof(*(nets->objs)) * (nets->count - (i + 1)));
if (VIR_REALLOC_N(nets->objs, nets->count - 1) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
nets->count--;
VIR_DELETE_ELEMENT(nets->objs, i, nets->count);
break;
}
virNetworkObjUnlock(nets->objs[i]);
@ -900,13 +887,17 @@ virNetworkDNSHostDefParseXML(const char *networkName,
if (cur->type == XML_ELEMENT_NODE &&
xmlStrEqual(cur->name, BAD_CAST "hostname")) {
if (cur->children != NULL) {
if (VIR_REALLOC_N(def->names, def->nnames + 1) < 0)
goto error;
def->names[def->nnames++] = (char *)xmlNodeGetContent(cur);
if (!def->names[def->nnames - 1]) {
char *name = (char *) xmlNodeGetContent(cur);
if (!name) {
virReportError(VIR_ERR_XML_DETAIL,
_("Missing hostname in network '%s' DNS HOST record"),
networkName);
goto error;
}
if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0) {
VIR_FREE(name);
goto error;
}
}
}

4
src/conf/network_conf.h
View File

@ -108,7 +108,7 @@ typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef;
typedef virNetworkDNSHostDef *virNetworkDNSHostDefPtr;
struct _virNetworkDNSHostDef {
virSocketAddr ip;
int nnames;
size_t nnames;
char **names;
};
@ -294,7 +294,7 @@ struct _virNetworkObj {
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
struct _virNetworkObjList {
unsigned int count;
size_t count;
virNetworkObjPtr *objs;
};

16
src/conf/node_device_conf.c
View File

@ -186,15 +186,13 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
return NULL;
}
virNodeDeviceObjLock(device);
device->def = def;
if (VIR_REALLOC_N(devs->objs, devs->count+1) < 0) {
device->def = NULL;
if (VIR_APPEND_ELEMENT_COPY(devs->objs, devs->count, device) < 0){
virNodeDeviceObjUnlock(device);
virNodeDeviceObjFree(device);
return NULL;
}
devs->objs[devs->count++] = device;
device->def = def;
return device;
@ -213,15 +211,7 @@ void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
virNodeDeviceObjUnlock(dev);
virNodeDeviceObjFree(devs->objs[i]);
if (i < (devs->count - 1))
memmove(devs->objs + i, devs->objs + i + 1,
sizeof(*(devs->objs)) * (devs->count - (i + 1)));
if (VIR_REALLOC_N(devs->objs, devs->count - 1) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
devs->count--;
VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
break;
}
virNodeDeviceObjUnlock(dev);

2
src/conf/node_device_conf.h
View File

@ -204,7 +204,7 @@ struct _virNodeDeviceObj {
typedef struct _virNodeDeviceObjList virNodeDeviceObjList;
typedef virNodeDeviceObjList *virNodeDeviceObjListPtr;
struct _virNodeDeviceObjList {
unsigned int count;
size_t count;
virNodeDeviceObjPtr *objs;
};

33
src/conf/nwfilter_conf.c
View File

@ -399,15 +399,13 @@ virNWFilterRuleDefAddString(virNWFilterRuleDefPtr nwf,
const char *string,
size_t maxstrlen)
{
if (VIR_REALLOC_N(nwf->strings, nwf->nstrings+1) < 0)
return NULL;
char *tmp;
if (VIR_STRNDUP(nwf->strings[nwf->nstrings], string, maxstrlen) < 0)
return NULL;
if (VIR_STRNDUP(tmp, string, maxstrlen) < 0 ||
VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0)
VIR_FREE(tmp);
nwf->nstrings++;
return nwf->strings[nwf->nstrings-1];
return tmp;
}
@ -425,15 +423,7 @@ virNWFilterObjRemove(virNWFilterObjListPtr nwfilters,
virNWFilterObjUnlock(nwfilters->objs[i]);
virNWFilterObjFree(nwfilters->objs[i]);
if (i < (nwfilters->count - 1))
memmove(nwfilters->objs + i, nwfilters->objs + i + 1,
sizeof(*(nwfilters->objs)) * (nwfilters->count - (i + 1)));
if (VIR_REALLOC_N(nwfilters->objs, nwfilters->count - 1) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
nwfilters->count--;
VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count);
break;
}
virNWFilterObjUnlock(nwfilters->objs[i]);
@ -2593,11 +2583,11 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
}
if (entry->rule || entry->include) {
if (VIR_REALLOC_N(ret->filterEntries, ret->nentries+1) < 0) {
if (VIR_APPEND_ELEMENT_COPY(ret->filterEntries,
ret->nentries, entry) < 0) {
virNWFilterEntryFree(entry);
goto cleanup;
}
ret->filterEntries[ret->nentries++] = entry;
} else
virNWFilterEntryFree(entry);
}
@ -3029,15 +3019,14 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
}
virNWFilterObjLock(nwfilter);
nwfilter->active = 0;
nwfilter->def = def;
if (VIR_REALLOC_N(nwfilters->objs, nwfilters->count + 1) < 0) {
nwfilter->def = NULL;
if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs,
nwfilters->count, nwfilter) < 0) {
virNWFilterObjUnlock(nwfilter);
virNWFilterObjFree(nwfilter);
return NULL;
}
nwfilters->objs[nwfilters->count++] = nwfilter;
nwfilter->def = def;
return nwfilter;
}

8
src/conf/nwfilter_conf.h
View File

@ -476,7 +476,7 @@ struct _virNWFilterRuleDef {
size_t nVarAccess;
virNWFilterVarAccessPtr *varAccess;
int nstrings;
size_t nstrings;
char **strings;
};
@ -524,7 +524,7 @@ struct _virNWFilterDef {
char *chainsuffix;
virNWFilterChainPriority chainPriority;
int nentries;
size_t nentries;
virNWFilterEntryPtr *filterEntries;
};
@ -547,7 +547,7 @@ struct _virNWFilterObj {
typedef struct _virNWFilterObjList virNWFilterObjList;
typedef virNWFilterObjList *virNWFilterObjListPtr;
struct _virNWFilterObjList {
unsigned int count;
size_t count;
virNWFilterObjPtr *objs;
};
@ -572,7 +572,7 @@ typedef virNWFilterTechDriver *virNWFilterTechDriverPtr;
typedef struct _virNWFilterRuleInst virNWFilterRuleInst;
typedef virNWFilterRuleInst *virNWFilterRuleInstPtr;
struct _virNWFilterRuleInst {
int ndata;
size_t ndata;
void **data;
virNWFilterTechDriverPtr techdriver;
};

12
src/conf/nwfilter_params.c
View File

@ -256,13 +256,7 @@ virNWFilterVarValueDelNthValue(virNWFilterVarValuePtr val, unsigned int pos)
case NWFILTER_VALUE_TYPE_ARRAY:
if (pos < val->u.array.nValues) {
VIR_FREE(val->u.array.values[pos]);
val->u.array.nValues--;
if (pos < val->u.array.nValues)
memmove(&val->u.array.values[pos],
&val->u.array.values[pos + 1],
sizeof(val->u.array.values[0]) *
(val->u.array.nValues - pos));
VIR_DELETE_ELEMENT(val->u.array.values, pos, val->u.array.nValues);
return 0;
}
break;
@ -642,11 +636,11 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
if (VIR_STRDUP(newName, name) < 0)
return -1;
if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) {
if (VIR_APPEND_ELEMENT_COPY(table->names,
table->nNames, newName) < 0) {
VIR_FREE(newName);
return -1;
}
table->names[table->nNames++] = newName;
}
if (virHashAddEntry(table->hashTable, name, val) < 0) {

2
src/conf/nwfilter_params.h
View File

@ -67,7 +67,7 @@ typedef virNWFilterHashTable *virNWFilterHashTablePtr;
struct _virNWFilterHashTable {
virHashTablePtr hashTable;
int nNames;
size_t nNames;
char **names;
};

14
src/conf/object_event.c
View File

@ -266,8 +266,7 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn,
static int
virObjectEventCallbackListPurgeMarked(virObjectEventCallbackListPtr cbList)
{
int old_count = cbList->count;
int n;
size_t n;
for (n = 0; n < cbList->count; n++) {
if (cbList->callbacks[n]->deleted) {
virFreeCallback freecb = cbList->callbacks[n]->freecb;
@ -276,19 +275,10 @@ virObjectEventCallbackListPurgeMarked(virObjectEventCallbackListPtr cbList)
virObjectUnref(cbList->callbacks[n]->conn);
VIR_FREE(cbList->callbacks[n]);
if (n < (cbList->count - 1))
memmove(cbList->callbacks + n,
cbList->callbacks + n + 1,
sizeof(*(cbList->callbacks)) *
(cbList->count - (n + 1)));
cbList->count--;
VIR_DELETE_ELEMENT(cbList->callbacks, n, cbList->count);
n--;
}
}
if (cbList->count < old_count &&
VIR_REALLOC_N(cbList->callbacks, cbList->count) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
return 0;
}

16
src/conf/storage_conf.c
View File

@ -458,15 +458,7 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
virStoragePoolObjUnlock(pools->objs[i]);
virStoragePoolObjFree(pools->objs[i]);
if (i < (pools->count - 1))
memmove(pools->objs + i, pools->objs + i + 1,
sizeof(*(pools->objs)) * (pools->count - (i + 1)));
if (VIR_REALLOC_N(pools->objs, pools->count - 1) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
pools->count--;
VIR_DELETE_ELEMENT(pools->objs, i, pools->count);
break;
}
virStoragePoolObjUnlock(pools->objs[i]);
@ -1782,15 +1774,13 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
}
virStoragePoolObjLock(pool);
pool->active = 0;
pool->def = def;
if (VIR_REALLOC_N(pools->objs, pools->count+1) < 0) {
pool->def = NULL;
if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, pool) < 0) {
virStoragePoolObjUnlock(pool);
virStoragePoolObjFree(pool);
return NULL;
}
pools->objs[pools->count++] = pool;
pool->def = def;
return pool;
}

4
src/qemu/qemu_driver.c
View File

@ -4301,7 +4301,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
int ret = -1;
qemuDomainObjPrivatePtr priv;
bool doReset = false;
int newVcpuPinNum = 0;
size_t newVcpuPinNum = 0;
virDomainVcpuPinDefPtr *newVcpuPin = NULL;
virBitmapPtr pcpumap = NULL;
virQEMUDriverConfigPtr cfg = NULL;
@ -4575,7 +4575,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
int ret = -1;
qemuDomainObjPrivatePtr priv;
bool doReset = false;
int newVcpuPinNum = 0;
size_t newVcpuPinNum = 0;
virDomainVcpuPinDefPtr *newVcpuPin = NULL;
virBitmapPtr pcpumap = NULL;
virQEMUDriverConfigPtr cfg = NULL;