Re: [Qemu-devel] [PATCH 3/4] block/gluster: using new qapi schema

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2539 bytes --]

On 11/12/2015 03:22 AM, Prasanna Kumar Kalever wrote:
> this patch adds GlusterConf to qapi/block-core.json
> 
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> ---
>  block/gluster.c      | 104 +++++++++++++++++++++++++--------------------------
>  qapi/block-core.json |  60 +++++++++++++++++++++++++++--
>  2 files changed, 109 insertions(+), 55 deletions(-)

One more comment:

> -static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
> +static int qemu_gluster_parseuri(BlockdevOptionsGluster **pgconf,
> +                                 const char *filename)
>  {
> +    BlockdevOptionsGluster *gconf;
>      URI *uri;
>      QueryParams *qp = NULL;
>      bool is_unix = false;
> @@ -155,20 +141,24 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
>          return -EINVAL;

If we hit this early return, then *pgconf was never assigned...


> +static struct glfs *qemu_gluster_init(BlockdevOptionsGluster **pgconf,
> +                                      const char *filename, Error **errp)
>  {
> -    struct glfs *glfs = NULL;
> +    struct glfs *glfs;
>      int ret;
>      int old_errno;
> +    BlockdevOptionsGluster *gconf;

but here, gconf is uninitialized,

>  
> -    ret = qemu_gluster_parseuri(gconf, filename);
> +    ret = qemu_gluster_parseuri(&gconf, filename);
>      if (ret < 0) {
>          error_setg(errp, "Usage: file=gluster[+transport]://[host[:port]]/"
>                           "volume/path[?socket=...]");

which means we can goto out with it uninitialized...

> @@ -224,8 +225,9 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
>          goto out;
>      }
>  
> -    ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->host,
> -            gconf->port);
> +    ret = glfs_set_volfile_server(glfs,
> +                                  GlusterTransport_lookup[gconf->server->transport],
> +                                  gconf->server->host, gconf->server->port);
>      if (ret < 0) {
>          goto out;
>      }

...vs. here where we can goto out with it initialized.

So whatever solution you use to plug the leak must be careful to not
free uninitialized memory.  Easiest solution - initialize gconf to NULL
before qemu_gluster_parseuri (or else go back to a *gconf parameter
rather than **pgconf).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]