[libvirt] [PATCH 11/20] phyp: Remove the last instance of stack allocating a 4kb volume key

[Daniel P. Berrange]

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 :|