Le Tuesday 17 June 2014 11:24:09, Peter Krempa a écrit :
In virVBoxSnapshotConfRemoveFakeDisks and virVBoxSnapshotConfDiskIsInMediaRegistry the disk array constructed from all the disks would be leaked at the end of the function and on allocation errors. Also the temporary disk list would be leaked.
Add a cleanup section and free the memory properly.
Found by coverity. --- src/vbox/vbox_snapshot_conf.c | 54 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 69c7e42..49fd2b6 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -1471,28 +1471,45 @@ virVBoxSnapshotConfRemoveFakeDisks(virVBoxSnapshotConfMachinePtr machine) size_t diskSize = 0; virVBoxSnapshotConfHardDiskPtr *tempList = NULL; virVBoxSnapshotConfHardDiskPtr *diskList = NULL; + if (VIR_ALLOC_N(diskList, 0) < 0) - return ret; + return -1;
for (i = 0; i < machine->mediaRegistry->ndisks; i++) { tempSize = virVBoxSnapshotConfAllChildren(machine->mediaRegistry->disks[i], &tempList); if (VIR_EXPAND_N(diskList, diskSize, tempSize) < 0) - return ret; - for (j = 0; j < tempSize; j++) { + goto cleanup; + + for (j = 0; j < tempSize; j++) diskList[diskSize - tempSize + j] = tempList[j]; - } + + VIR_FREE(tempList); } + for (i = 0; i < diskSize; i++) { if (strstr(diskList[i]->location, "fake") != NULL) { if (virVBoxSnapshotConfRemoveHardDisk(machine->mediaRegistry, diskList[i]->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to remove hard disk %s from media registry"), diskList[i]->location); - return ret; + goto cleanup; } } } + ret = 0; + + cleanup: + for (i = 0; i < diskSize; i++) + virVboxSnapshotConfHardDiskFree(diskList[i]); + VIR_FREE(diskList); + + if (tempList) { + for (i = 0; i < tempSize; i++) + virVboxSnapshotConfHardDiskFree(tempList[i]); + VIR_FREE(tempList); + } Same comments than the previous patch + return ret; }
@@ -1513,24 +1530,41 @@ virVBoxSnapshotConfDiskIsInMediaRegistry(virVBoxSnapshotConfMachinePtr machine, size_t diskSize = 0; virVBoxSnapshotConfHardDiskPtr *tempList = NULL; virVBoxSnapshotConfHardDiskPtr *diskList = NULL; + if (VIR_ALLOC_N(diskList, 0) < 0) - return ret; + return -1;
for (i = 0; i < machine->mediaRegistry->ndisks; i++) { tempSize = virVBoxSnapshotConfAllChildren(machine->mediaRegistry->disks[i], &tempList); if (VIR_EXPAND_N(diskList, diskSize, tempSize) < 0) - return ret; - for (j = 0; j < tempSize; j++) { + goto cleanup; + + for (j = 0; j < tempSize; j++) diskList[diskSize - tempSize + j] = tempList[j]; - } + + VIR_FREE(tempList); } + for (i = 0; i < diskSize; i++) { if (STREQ(diskList[i]->location, location)) { ret = 1; - return ret; + goto cleanup; } } + ret = 0; + + cleanup: + for (i = 0; i < diskSize; i++) + virVboxSnapshotConfHardDiskFree(diskList[i]); + VIR_FREE(diskList); + + if (tempList) { + for (i = 0; i < tempSize; i++) + virVboxSnapshotConfHardDiskFree(tempList[i]); + VIR_FREE(tempList); + } Same comments than the previous patch + return ret; }
-- Yohan BELLEGUIC Software Engineer - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 05