On 2012年05月04日 19:54, Stefan Berger wrote:

Error: RESOURCE_LEAK: /libvirt/src/vmx/vmx.c:2431: alloc_fn: Calling allocation function "calloc". /libvirt/src/vmx/vmx.c:2431: var_assign: Assigning: "networkName" = storage returned from "calloc(1UL, 1UL)". /libvirt/src/vmx/vmx.c:2495: leaked_storage: Variable "networkName" going out of scope leaks the storage it points to.

--- src/vmx/vmx.c | 1 + 1 file changed, 1 insertion(+)

Index: libvirt-acl/src/vmx/vmx.c =================================================================== --- libvirt-acl.orig/src/vmx/vmx.c +++ libvirt-acl/src/vmx/vmx.c @@ -2485,6 +2485,7 @@ virVMXParseEthernet(virConfPtr conf, int *def = NULL; }

+ VIR_FREE(networkName); VIR_FREE(connectionType); VIR_FREE(addressType); VIR_FREE(generatedAddress);

ACK Osier

On 2012年05月04日 19:54, Stefan Berger wrote:

Error: RESOURCE_LEAK: /libvirt/src/qemu/qemu_driver.c:6968: alloc_fn: Calling allocation function "calloc". /libvirt/src/qemu/qemu_driver.c:6968: var_assign: Assigning: "nodeset" = storage returned from "calloc(1UL, 1UL)". /libvirt/src/qemu/qemu_driver.c:6977: noescape: Variable "nodeset" is not freed or pointed-to in function "virTypedParameterAssign". /libvirt/src/qemu/qemu_driver.c:6997: leaked_storage: Variable "nodeset" going out of scope leaks the storage it points to.

--- src/qemu/qemu_driver.c | 4 ++++ 1 file changed, 4 insertions(+)

Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -6991,6 +6991,9 @@ qemuDomainGetNumaParameters(virDomainPtr if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET, VIR_TYPED_PARAM_STRING, nodeset)< 0) goto cleanup; + + nodeset = NULL; + break;

default: @@ -7004,6 +7007,7 @@ qemuDomainGetNumaParameters(virDomainPtr ret = 0;

cleanup: + VIR_FREE(nodeset); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm);

--

ACK Osier

On 2012年05月04日 19:54, Stefan Berger wrote:

Error: RESOURCE_LEAK: /libvirt/tests/qemuxml2argvtest.c:47: alloc_arg: Calling allocation function "virAlloc" on "ret". /libvirt/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /libvirt/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /libvirt/tests/qemuxml2argvtest.c:54: leaked_storage: Variable "ret" going out of scope leaks the storage it points to.

--- tests/qemuxml2argvtest.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

Index: libvirt-acl/tests/qemuxml2argvtest.c =================================================================== --- libvirt-acl.orig/tests/qemuxml2argvtest.c +++ libvirt-acl/tests/qemuxml2argvtest.c @@ -50,8 +50,10 @@ fakeSecretLookupByUsage(virConnectPtr co ret->magic = VIR_SECRET_MAGIC; ret->refs = 1; ret->usageID = strdup(usageID); - if (!ret->usageID) + if (!ret->usageID) { + VIR_FREE(ret); return NULL; + } ret->conn = conn; conn->refs++; return ret;

ACK Osier

On 05/04/2012 05:54 AM, Stefan Berger wrote:

Error: STRING_NULL: /libvirt/src/node_device/node_device_linux_sysfs.c:80: string_null_argument: Function "saferead" does not terminate string "*buf". /libvirt/src/util/util.c:101: string_null_argument: Function "read" fills array "*buf" with a non-terminated string. /libvirt/src/node_device/node_device_linux_sysfs.c:87: string_null: Passing unterminated string "buf" to a function expecting a null-terminated string.

--- src/node_device/node_device_linux_sysfs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)

Index: libvirt-acl/src/node_device/node_device_linux_sysfs.c =================================================================== --- libvirt-acl.orig/src/node_device/node_device_linux_sysfs.c +++ libvirt-acl/src/node_device/node_device_linux_sysfs.c @@ -69,20 +69,21 @@ out: int read_wwn_linux(int host, const char *file, char **wwn) { char *p = NULL; - int fd = -1, retval = 0; - char buf[64]; + int fd = -1, retval = 0, len; + char buf[65];

Here, I would write: char buf[65] = "";

if (open_wwn_file(LINUX_SYSFS_FC_HOST_PREFIX, host, file, &fd) < 0) { goto out; }

- memset(buf, 0, sizeof(buf));

Then the memset is not necessary (the initialization took care of that instead).

- if (saferead(fd, buf, sizeof(buf)) < 0) { + len = saferead(fd, buf, sizeof(buf) - 1);

You are correct that you need to use sizeof(buf) - 1. But if you guarantee that buf was all NUL on entry, then you don't have to worry about the resulting len,...

+ if (len < 0) { retval = -1; VIR_DEBUG("Failed to read WWN for host%d '%s'", host, file); goto out; } + buf[len] = '\0';

and therefore don't need this trailing assignment. We definitely have an off-by-one bug here, but I don't think we need quite as many changed to fix the issue as what you have here. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org