3:07 p.m.
virStorageBackendRBDRefreshPool() first allocates an array big enough
to hold 1024 names, then calls rbd_list(), which returns ERANGE if the
array isn't big enough. When that happens, the VIR_ALLOC_N is called
again with a larger size. Unfortunately, the original array isn't
freed before allocating a new one.
---
src/storage/storage_backend_rbd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 8a0e517..e815192 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -317,6 +317,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
VIR_WARN("%s", _("A problem occurred while listing RBD
images"));
goto cleanup;
}
+ VIR_FREE(names);
}
for (i = 0, name = names; name < names + max_size; i++) {
--
1.7.11.7
5:28 p.m.
On 03/18/2013 02:07 PM, Laine Stump wrote:
virStorageBackendRBDRefreshPool() first allocates an array big
enough
to hold 1024 names, then calls rbd_list(), which returns ERANGE if the
array isn't big enough. When that happens, the VIR_ALLOC_N is called
again with a larger size. Unfortunately, the original array isn't
freed before allocating a new one.
---
src/storage/storage_backend_rbd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 8a0e517..e815192 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -317,6 +317,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
VIR_WARN("%s", _("A problem occurred while listing RBD
images"));
goto cleanup;
}
+ VIR_FREE(names);
This works, but is possibly less efficient than using VIR_REALLOC_N
instead of VIR_ALLOC_N in the first place. ACK, since it's not on the
hot path.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:44 p.m.
On 03/18/2013 06:28 PM, Eric Blake wrote:
On 03/18/2013 02:07 PM, Laine Stump wrote:
> virStorageBackendRBDRefreshPool() first allocates an array big enough
> to hold 1024 names, then calls rbd_list(), which returns ERANGE if the
> array isn't big enough. When that happens, the VIR_ALLOC_N is called
> again with a larger size. Unfortunately, the original array isn't
> freed before allocating a new one.
> ---
> src/storage/storage_backend_rbd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index 8a0e517..e815192 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
> @@ -317,6 +317,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
> VIR_WARN("%s", _("A problem occurred while listing RBD
images"));
> goto cleanup;
> }
> + VIR_FREE(names);
This works, but is possibly less efficient than using VIR_REALLOC_N
instead of VIR_ALLOC_N in the first place.
I had thought of that, but figured that internally it would likely be
the same operation as a free + new malloc, but would also do a copy from
the old region to new, which is pointless in this case, since the old
memory hasn't been set to anything and will be immediately overwritten
anyway.
ACK, since it's not on the
hot path.
I'm pushing as is.
5:32 p.m.
On 03/18/2013 04:07 PM, Laine Stump wrote:
virStorageBackendRBDRefreshPool() first allocates an array big
enough
to hold 1024 names, then calls rbd_list(), which returns ERANGE if the
array isn't big enough. When that happens, the VIR_ALLOC_N is called
again with a larger size. Unfortunately, the original array isn't
freed before allocating a new one.
---
src/storage/storage_backend_rbd.c | 1 +
1 file changed, 1 insertion(+)
ACK
Interesting only by upping the Coverity analysis level would this show
up in a coverity analysis (along with 400 more).
John
4262
days inactive
4267
days old
3 comments
3 participants
participants (3)
-
Eric Blake -
John Ferlan -
Laine Stump