All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <elder@inktank.com>
To: ceph-devel@vger.kernel.org
Subject: [PATCH 5/6] rbd: fix leak of format 2 snapshot names
Date: Fri, 26 Apr 2013 07:06:41 -0500	[thread overview]
Message-ID: <517A6DD1.2080102@inktank.com> (raw)
In-Reply-To: <517A6D39.80000@inktank.com>

When the snapshot context for an rbd device gets updated (or the
initial one is recorded) a a list of snapshot structures is created
to represent them, one entry per snapshot.  Each entry includes a
dynamically-allocated copy of the snapshot name.

Currently the name is allocated in rbd_snap_create(), as a duplicate
of the passed-in name.

For format 1 images, the snapshot name provided is just a pointer to
an existing name.  But for format 2 images, the passed-in name is
already dynamically allocated, and in the the process of duplicating
it here we are leaking the passed-in name.

Fix this by dynamically allocating the name for format 1 snapshots
also, and then stop allocating a duplicate in rbd_snap_create().

Change rbd_dev_v1_snap_info() so none of its parameters is
side-effected unless it's going to return success.

This is part of:
    http://tracker.ceph.com/issues/4803

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 916741b..2b5ba50 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3427,30 +3427,23 @@ static struct rbd_snap *rbd_snap_create(struct
rbd_device *rbd_dev,
 						u64 snap_features)
 {
 	struct rbd_snap *snap;
-	int ret;

 	snap = kzalloc(sizeof (*snap), GFP_KERNEL);
 	if (!snap)
 		return ERR_PTR(-ENOMEM);

-	ret = -ENOMEM;
-	snap->name = kstrdup(snap_name, GFP_KERNEL);
-	if (!snap->name)
-		goto err;
-
+	snap->name = snap_name;
 	snap->id = snap_id;
 	snap->size = snap_size;
 	snap->features = snap_features;

 	return snap;
-
-err:
-	kfree(snap->name);
-	kfree(snap);
-
-	return ERR_PTR(ret);
 }

+/*
+ * Returns a dynamically-allocated snapshot name if successful, or a
+ * pointer-coded error otherwise.
+ */
 static char *rbd_dev_v1_snap_info(struct rbd_device *rbd_dev, u32 which,
 		u64 *snap_size, u64 *snap_features)
 {
@@ -3458,15 +3451,19 @@ static char *rbd_dev_v1_snap_info(struct
rbd_device *rbd_dev, u32 which,

 	rbd_assert(which < rbd_dev->header.snapc->num_snaps);

-	*snap_size = rbd_dev->header.snap_sizes[which];
-	*snap_features = 0;	/* No features for v1 */
-
 	/* Skip over names until we find the one we are looking for */

 	snap_name = rbd_dev->header.snap_names;
 	while (which--)
 		snap_name += strlen(snap_name) + 1;

+	snap_name = kstrdup(snap_name, GFP_KERNEL);
+	if (!snap_name);
+		return ERR_PTR(-ENOMEM);
+
+	*snap_size = rbd_dev->header.snap_sizes[which];
+	*snap_features = 0;	/* No features for v1 */
+
 	return snap_name;
 }

-- 
1.7.9.5


  parent reply	other threads:[~2013-04-26 12:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-26 12:04 [PATCH 0/6] rbd: cleanup and plug leaks Alex Elder
2013-04-26 12:05 ` [PATCH 1/6] rbd: fix leak of snapshots during initial probe Alex Elder
2013-04-29 15:12   ` Josh Durgin
2013-04-26 12:05 ` [PATCH 2/6] rbd: make snap_size order parameter optional Alex Elder
2013-04-29 15:14   ` Josh Durgin
2013-04-26 12:06 ` [PATCH 3/6] rbd: only update values on snap_info success Alex Elder
2013-04-29 15:15   ` Josh Durgin
2013-04-26 12:06 ` [PATCH 4/6] rbd: rename __rbd_add_snap_dev() Alex Elder
2013-04-29 15:15   ` Josh Durgin
2013-04-26 12:06 ` Alex Elder [this message]
2013-04-26 17:40   ` [PATCH 5/6, v2] rbd: fix leak of format 2 snapshot names Alex Elder
2013-04-29 15:16   ` [PATCH 5/6] " Josh Durgin
2013-04-29 15:18     ` Josh Durgin
2013-04-26 12:06 ` [PATCH 6/6] rbd: use rbd_obj_method_sync() return value Alex Elder
2013-04-29 15:22   ` Josh Durgin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=517A6DD1.2080102@inktank.com \
    --to=elder@inktank.com \
    --cc=ceph-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.