On Sun, Apr 03, 2011 at 11:21:24AM +0200, Matthias Bolte wrote:
> ---
> src/phyp/phyp_driver.c | 13 +++++++++++--
> 1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index fe2e99d..76207c2 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
> @@ -2362,13 +2362,22 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname,
> static virStorageVolPtr
> phypVolumeLookupByName(virStoragePoolPtr pool, const char *volname)
> {
> + char *key;
> + virStorageVolPtr vol;
>
> - char key[MAX_KEY_SIZE];
> + if (VIR_ALLOC_N(key, MAX_KEY_SIZE) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
>
> if (phypVolumeGetKey(pool->conn, key, volname) == -1)
> return NULL;
>
> - return virGetStorageVol(pool->conn, pool->name, volname, key);
> + vol = virGetStorageVol(pool->conn, pool->name, volname, key);
> +
> + VIR_FREE(key);
> +
> + return vol;
> }
I think the signature of phypVolumeGetKey() is rather dangerous - it is
blindly assuming the caller allocates MAX_KEY_SIZE for 'key'. The
phypVolumeGetKey knows exactly how long the key it has is, so it'd be
better for it to allocate the buffer itself & return it to the callers
I realize this isn't a new problem from your patch, but I reckon we
should fix it here.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list