External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 13:45:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

virmacmap: Don't use hash table dataFree callback

Browse Source 
Due to nature of operations we do over the string list (more
precisely due to how virStringListRemove() works), it is not the
best idea to use dataFree callback. Problem is, on MAC address
remove, the string list remove function modifies the original
list in place. Then, virHashUpdateEntry() is called which frees
all the data stored in the list rendering @newMacsList point to
freed data.

==16002== Invalid read of size 8
==16002==    at 0x50BC083: virFree (viralloc.c:582)
==16002==    by 0x513DC39: virStringListFree (virstring.c:251)
==16002==    by 0x51089B4: virMacMapHashFree (virmacmap.c:67)
==16002==    by 0x50EF30B: virHashAddOrUpdateEntry (virhash.c:352)
==16002==    by 0x50EF4FD: virHashUpdateEntry (virhash.c:415)
==16002==    by 0x5108BED: virMacMapRemoveLocked (virmacmap.c:129)
==16002==    by 0x51092D5: virMacMapRemove (virmacmap.c:346)
==16002==    by 0x402F02: testMACRemove (virmacmaptest.c:107)
==16002==    by 0x403F15: virTestRun (testutils.c:180)
==16002==    by 0x4032C4: mymain (virmacmaptest.c:205)
==16002==    by 0x405A3B: virTestMain (testutils.c:992)
==16002==    by 0x403D87: main (virmacmaptest.c:237)
==16002==  Address 0xdd5a4d0 is 0 bytes inside a block of size 24 free'd
==16002==    at 0x4C2AD6F: realloc (vg_replace_malloc.c:693)
==16002==    by 0x50BB99B: virReallocN (viralloc.c:245)
==16002==    by 0x513DC0B: virStringListRemove (virstring.c:235)
==16002==    by 0x5108BA6: virMacMapRemoveLocked (virmacmap.c:124)
==16002==    by 0x51092D5: virMacMapRemove (virmacmap.c:346)
==16002==    by 0x402F02: testMACRemove (virmacmaptest.c:107)
==16002==    by 0x403F15: virTestRun (testutils.c:180)
==16002==    by 0x4032C4: mymain (virmacmaptest.c:205)
==16002==    by 0x405A3B: virTestMain (testutils.c:992)
==16002==    by 0x403D87: main (virmacmaptest.c:237)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Michal Privoznik 2017-01-02 10:35:33 +01:00
parent 806582a5d1
commit 5dc6169bc8
1 changed files with 15 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/util/virmacmap.c
View File

@ -53,18 +53,22 @@ struct virMacMap {
static virClassPtr virMacMapClass;
static void
virMacMapDispose(void *obj)
static int
virMacMapHashFree(void *payload,
const void *name ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
virMacMapPtr mgr = obj;
virHashFree(mgr->macs);
virStringListFree(payload);
return 0;
}
static void
virMacMapHashFree(void *payload, const void *name ATTRIBUTE_UNUSED)
virMacMapDispose(void *obj)
{
virStringListFree(payload);
virMacMapPtr mgr = obj;
virHashForEach(mgr->macs, virMacMapHashFree, NULL);
virHashFree(mgr->macs);
}
@ -88,19 +92,20 @@ virMacMapAddLocked(virMacMapPtr mgr,
const char *mac)
{
int ret = -1;
const char **macsList = NULL;
char **macsList = NULL;
char **newMacsList = NULL;
if ((macsList = virHashLookup(mgr->macs, domain)) &&
virStringListHasString(macsList, mac)) {
virStringListHasString((const char**) macsList, mac)) {
ret = 0;
goto cleanup;
}
if (!(newMacsList = virStringListAdd(macsList, mac)) ||
if (!(newMacsList = virStringListAdd((const char **) macsList, mac)) ||
virHashUpdateEntry(mgr->macs, domain, newMacsList) < 0)
goto cleanup;
newMacsList = NULL;
virStringListFree(macsList);
ret = 0;
cleanup:
@ -303,8 +308,7 @@ virMacMapNew(const char *file)
return NULL;
virObjectLock(mgr);
if (!(mgr->macs = virHashCreate(VIR_MAC_HASH_TABLE_SIZE,
virMacMapHashFree)))
if (!(mgr->macs = virHashCreate(VIR_MAC_HASH_TABLE_SIZE, NULL)))
goto error;
if (file &&