From patchwork Fri Aug 30 00:57:06 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Josh Durgin X-Patchwork-Id: 2851701 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 245D49F2F4 for ; Fri, 30 Aug 2013 01:09:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 332EA202B8 for ; Fri, 30 Aug 2013 01:09:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 23A1320278 for ; Fri, 30 Aug 2013 01:09:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753265Ab3H3BJe (ORCPT ); Thu, 29 Aug 2013 21:09:34 -0400 Received: from mail.hq.newdream.net ([66.33.206.127]:39827 "EHLO mail.hq.newdream.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752103Ab3H3BJd (ORCPT ); Thu, 29 Aug 2013 21:09:33 -0400 Received: from mail.hq.newdream.net (localhost [127.0.0.1]) by mail.hq.newdream.net (Postfix) with ESMTP id 60A0626391 for ; Thu, 29 Aug 2013 18:13:27 -0700 (PDT) Received: from vit.ops.newdream.net (unknown [38.122.20.226]) by mail.hq.newdream.net (Postfix) with ESMTPSA id 4C2CD26389 for ; Thu, 29 Aug 2013 18:13:27 -0700 (PDT) From: Josh Durgin To: ceph-devel@vger.kernel.org Subject: [PATCH v2 1/3] rbd: fix use-after free of rbd_dev->disk Date: Thu, 29 Aug 2013 17:57:06 -0700 Message-Id: <1377824228-14632-1-git-send-email-josh.durgin@inktank.com> X-Mailer: git-send-email 1.7.2.5 In-Reply-To: <1377757447-23515-4-git-send-email-josh.durgin@inktank.com> References: <1377757447-23515-4-git-send-email-josh.durgin@inktank.com> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Removing a device deallocates the disk, unschedules the watch, and finally cleans up the rbd_dev structure. rbd_dev_refresh(), called from the watch callback, updates the disk size and rbd_dev structure. With no locking between them, rbd_dev_refresh() may use the device or rbd_dev after they've been freed. To fix this, add a shutting_down flag and a mutex protecting it to rbd_dev. Take the mutex and check whether the flag is set before using rbd_dev->disk. Move this disk-updating to a separate function as well. Fixes: http://tracker.ceph.com/issues/5636 Signed-off-by: Josh Durgin Reviewed-by: Alex Elder --- drivers/block/rbd.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index fef3687..8ab3362b 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -343,6 +343,9 @@ struct rbd_device { struct ceph_osd_event *watch_event; struct rbd_obj_request *watch_request; + bool shutting_down; /* rbd_remove() in progress */ + struct mutex shutdown_lock; /* protects shutting_down */ + struct rbd_spec *parent_spec; u64 parent_overlap; atomic_t parent_ref; @@ -3324,6 +3327,24 @@ static void rbd_exists_validate(struct rbd_device *rbd_dev) clear_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags); } +static void rbd_dev_update_size(struct rbd_device *rbd_dev) +{ + sector_t size; + + mutex_lock(&rbd_dev->shutdown_lock); + /* + * If the device is being removed, rbd_dev->disk has + * been destroyed, so don't try to update its size + */ + if (!rbd_dev->shutting_down) { + size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE; + dout("setting size to %llu sectors", (unsigned long long)size); + set_capacity(rbd_dev->disk, size); + revalidate_disk(rbd_dev->disk); + } + mutex_unlock(&rbd_dev->shutdown_lock); +} + static int rbd_dev_refresh(struct rbd_device *rbd_dev) { u64 mapping_size; @@ -3343,12 +3364,7 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev) up_write(&rbd_dev->header_rwsem); if (mapping_size != rbd_dev->mapping.size) { - sector_t size; - - size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE; - dout("setting size to %llu sectors", (unsigned long long)size); - set_capacity(rbd_dev->disk, size); - revalidate_disk(rbd_dev->disk); + rbd_dev_update_size(rbd_dev); } return ret; @@ -3656,6 +3672,8 @@ static struct rbd_device *rbd_dev_create(struct rbd_client *rbdc, atomic_set(&rbd_dev->parent_ref, 0); INIT_LIST_HEAD(&rbd_dev->node); init_rwsem(&rbd_dev->header_rwsem); + mutex_init(&rbd_dev->shutdown_lock); + rbd_dev->shutting_down = false; rbd_dev->spec = spec; rbd_dev->rbd_client = rbdc; @@ -5159,7 +5177,16 @@ static ssize_t rbd_remove(struct bus_type *bus, if (ret < 0 || already) return ret; + /* + * hold shutdown_lock while destroying the device so that + * device destruction will not race with device updates from + * the watch callback + */ + mutex_lock(&rbd_dev->shutdown_lock); + rbd_dev->shutting_down = true; rbd_bus_del_dev(rbd_dev); + mutex_unlock(&rbd_dev->shutdown_lock); + ret = rbd_dev_header_watch_sync(rbd_dev, false); if (ret) rbd_warn(rbd_dev, "failed to cancel watch event (%d)\n", ret);