Patchwork [v3] rbd: fix leak in qemu_rbd_open failure paths

login
register
mail settings
Submitter Sage Weil
Date Sept. 4, 2011, 4:19 p.m.
Message ID <Pine.LNX.4.64.1109040918340.28858@cobra.newdream.net>
Download mbox | patch
Permalink /patch/113290/
State New
Headers show

Comments

Sage Weil - Sept. 4, 2011, 4:19 p.m.
Fix leak of s->snap in failure path.  Simplify error paths for the whole
function.

Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Sage Weil <sage@newdream.net>
---
 block/rbd.c |   28 +++++++++++++---------------
 1 files changed, 13 insertions(+), 15 deletions(-)
Stefan Hajnoczi - Sept. 4, 2011, 6:20 p.m.
On Sun, Sep 4, 2011 at 5:19 PM, Sage Weil <sage@newdream.net> wrote:
> Fix leak of s->snap in failure path.  Simplify error paths for the whole
> function.
>
> Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Sage Weil <sage@newdream.net>
> ---
>  block/rbd.c |   28 +++++++++++++---------------
>  1 files changed, 13 insertions(+), 15 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Kevin Wolf - Sept. 5, 2011, 12:32 p.m.
Am 04.09.2011 18:19, schrieb Sage Weil:
> Fix leak of s->snap in failure path.  Simplify error paths for the whole
> function.
> 
> Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Sage Weil <sage@newdream.net>

This depends on "[PATCH v2] rbd: allow client id to be specified in
config string", which doesn't seem to apply any more and has coding
style issues. Please fix and send a rebased series containing all four
patches.

Kevin

Patch

diff --git a/block/rbd.c b/block/rbd.c
index 2763092..9f32211 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -427,10 +427,6 @@  static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int flags)
                            conf, sizeof(conf)) < 0) {
         return -EINVAL;
     }
-    s->snap = NULL;
-    if (snap_buf[0] != '\0') {
-        s->snap = g_strdup(snap_buf);
-    }
 
     clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
     r = rados_create(&s->cluster, clientname);
@@ -439,12 +435,16 @@  static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int flags)
         return r;
     }
 
+    s->snap = NULL;
+    if (snap_buf[0] != '\0') {
+        s->snap = g_strdup(snap_buf);
+    }
+
     if (strstr(conf, "conf=") == NULL) {
         r = rados_conf_read_file(s->cluster, NULL);
         if (r < 0) {
             error_report("error reading config file");
-            rados_shutdown(s->cluster);
-            return r;
+	    goto failed_shutdown;
         }
     }
 
@@ -452,31 +452,26 @@  static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int flags)
         r = qemu_rbd_set_conf(s->cluster, conf);
         if (r < 0) {
             error_report("error setting config options");
-            rados_shutdown(s->cluster);
-            return r;
+	    goto failed_shutdown;
         }
     }
 
     r = rados_connect(s->cluster);
     if (r < 0) {
         error_report("error connecting");
-        rados_shutdown(s->cluster);
-        return r;
+	goto failed_shutdown;
     }
 
     r = rados_ioctx_create(s->cluster, pool, &s->io_ctx);
     if (r < 0) {
         error_report("error opening pool %s", pool);
-        rados_shutdown(s->cluster);
-        return r;
+	goto failed_shutdown;
     }
 
     r = rbd_open(s->io_ctx, s->name, &s->image, s->snap);
     if (r < 0) {
         error_report("error reading header from %s", s->name);
-        rados_ioctx_destroy(s->io_ctx);
-        rados_shutdown(s->cluster);
-        return r;
+	goto failed_open;
     }
 
     bs->read_only = (s->snap != NULL);
@@ -497,8 +492,11 @@  static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int flags)
 
 failed:
     rbd_close(s->image);
+failed_open:
     rados_ioctx_destroy(s->io_ctx);
+failed_shutdown:
     rados_shutdown(s->cluster);
+    g_free(s->snap);
     return r;
 }