From: Alex Elder <elder@inktank.com>
To: ceph-devel@vger.kernel.org
Subject: [PATCH] rbd: fix cleanup in rbd_add()
Date: Mon, 13 May 2013 20:39:00 -0500 [thread overview]
Message-ID: <519195B4.1080802@inktank.com> (raw)
Bjorn Helgaas pointed out that a recent commit introduced a
use-after-free condition in an error path for rbd_add().
He correctly stated:
I think b536f69a3a5 "rbd: set up devices only for mapped images"
introduced a use-after-free error in rbd_add():
...
If rbd_dev_device_setup() returns an error, we call
rbd_dev_image_release(), which ultimately kfrees rbd_dev.
Then we call rbd_dev_destroy(), which references fields in
the already-freed rbd_dev struct before kfreeing it again.
The simple fix is to return the error code after the call to
rbd_dev_image_release().
Closer examination revealed that there's no need to clean up
ceph_opts or rbd_opts in that function, so fix that too.
Update some other comments that have also become out of date.
Reported-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index e8d4693..632dd35 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4699,8 +4699,10 @@ out:
return ret;
}
-/* Undo whatever state changes are made by v1 or v2 image probe */
-
+/*
+ * Undo whatever state changes are made by v1 or v2 header info
+ * call.
+ */
static void rbd_dev_unprobe(struct rbd_device *rbd_dev)
{
struct rbd_image_header *header;
@@ -4904,9 +4906,10 @@ static int rbd_dev_image_probe(struct rbd_device
*rbd_dev, bool mapping)
int tmp;
/*
- * Get the id from the image id object. If it's not a
- * format 2 image, we'll get ENOENT back, and we'll assume
- * it's a format 1 image.
+ * Get the id from the image id object. Unless there's an
+ * error, rbd_dev->spec->image_id will be filled in with
+ * a dynamically-allocated string, and rbd_dev->image_format
+ * will be set to either 1 or 2.
*/
ret = rbd_dev_image_id(rbd_dev);
if (ret)
@@ -4994,7 +4997,7 @@ static ssize_t rbd_add(struct bus_type *bus,
rc = PTR_ERR(rbdc);
goto err_out_args;
}
- ceph_opts = NULL; /* rbd_dev client now owns this */
+ ceph_opts = NULL; /* rbd_get_client() consumes this */
/* pick the pool */
osdc = &rbdc->client->osdc;
@@ -5033,14 +5036,14 @@ static ssize_t rbd_add(struct bus_type *bus,
return count;
rbd_dev_image_release(rbd_dev);
+
+ return rc;
+
err_out_rbd_dev:
rbd_dev_destroy(rbd_dev);
err_out_client:
rbd_put_client(rbdc);
err_out_args:
- if (ceph_opts)
- ceph_destroy_options(ceph_opts);
- kfree(rbd_opts);
rbd_spec_put(spec);
err_out_module:
module_put(THIS_MODULE);
--
1.7.9.5
next reply other threads:[~2013-05-14 1:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-14 1:39 Alex Elder [this message]
2013-05-16 21:29 ` [PATCH, v2] rbd: fix cleanup in rbd_add() Alex Elder
2013-05-17 1:36 ` Josh Durgin
2013-05-17 3:40 ` Alex Elder
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=519195B4.1080802@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.