From patchwork Wed Jun 11 12:06:42 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Bader X-Patchwork-Id: 358626 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 0FC7014009F; Wed, 11 Jun 2014 22:07:04 +1000 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1WuhIy-0002hA-Gd; Wed, 11 Jun 2014 12:07:00 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1WuhIk-0002dG-QP for kernel-team@lists.ubuntu.com; Wed, 11 Jun 2014 12:06:46 +0000 Received: from hsi-kbw-149-172-227-5.hsi13.kabel-badenwuerttemberg.de ([149.172.227.5] helo=canonical.com) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1WuhIk-0000dX-KF for kernel-team@lists.ubuntu.com; Wed, 11 Jun 2014 12:06:46 +0000 From: Stefan Bader To: kernel-team@lists.ubuntu.com Subject: [PATCH 4/4] blkfront: Lock blkfront_info when closing Date: Wed, 11 Jun 2014 14:06:42 +0200 Message-Id: <1402488402-3192-5-git-send-email-stefan.bader@canonical.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1402488402-3192-1-git-send-email-stefan.bader@canonical.com> References: <1402488402-3192-1-git-send-email-stefan.bader@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: Daniel Stodden The bdev .open/.release fops race against backend switches to Closing, handled by the XenBus thread. The original code attempted to serialize block device holders and xenbus only via bd_mutex. This is insufficient, the info->bd pointer may already be stale (or null) while xenbus tries to bump up the refcount. Protect blkfront_info with a dedicated mutex. Signed-off-by: Daniel Stodden Signed-off-by: Jeremy Fitzhardinge BugLink: http://bugs.launchpad.net/bugs/1326870 (backported from commit b70f5fa043b318659c936d8c3c696250e6528944 upstream) Signed-off-by: Stefan Bader --- drivers/block/xen-blkfront.c | 61 +++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 56a8701..9616342 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -76,6 +76,7 @@ static const struct block_device_operations xlvbd_block_fops; */ struct blkfront_info { + struct mutex mutex; struct xenbus_device *xbdev; struct gendisk *gd; int vdevice; @@ -803,7 +804,6 @@ again: return err; } - /** * Entry point to this code when a new device is created. Allocate the basic * structures and the ring buffer for communication with the backend, and @@ -835,6 +835,7 @@ static int blkfront_probe(struct xenbus_device *dev, return -ENOMEM; } + mutex_init(&info->mutex); info->xbdev = dev; info->vdevice = vdevice; info->connected = BLKIF_STATE_DISCONNECTED; @@ -950,6 +951,43 @@ static int blkfront_resume(struct xenbus_device *dev) return err; } +static void +blkfront_closing(struct blkfront_info *info) +{ + struct xenbus_device *xbdev = info->xbdev; + struct block_device *bdev = NULL; + + mutex_lock(&info->mutex); + + if (xbdev->state == XenbusStateClosing) { + mutex_unlock(&info->mutex); + return; + } + + if (info->gd) + bdev = bdget_disk(info->gd, 0); + + mutex_unlock(&info->mutex); + + if (!bdev) { + xenbus_frontend_closed(xbdev); + return; + } + + mutex_lock(&bdev->bd_mutex); + + if (info->users) { + xenbus_dev_error(xbdev, -EBUSY, + "Device in use; refusing to close"); + xenbus_switch_state(xbdev, XenbusStateClosing); + } else { + xlvbd_release_gendisk(info); + xenbus_frontend_closed(xbdev); + } + + mutex_unlock(&bdev->bd_mutex); + bdput(bdev); +} /* * Invoked when the backend is finally 'ready' (and has told produced @@ -1014,7 +1052,6 @@ static void backend_changed(struct xenbus_device *dev, enum xenbus_state backend_state) { struct blkfront_info *info = dev_get_drvdata(&dev->dev); - struct block_device *bd; dev_dbg(&dev->dev, "blkfront:backend_changed.\n"); @@ -1031,25 +1068,7 @@ static void backend_changed(struct xenbus_device *dev, break; case XenbusStateClosing: - if (info->gd == NULL) { - xenbus_frontend_closed(dev); - break; - } - bd = bdget_disk(info->gd, 0); - if (bd == NULL) - xenbus_dev_fatal(dev, -ENODEV, "bdget failed"); - - mutex_lock(&bd->bd_mutex); - if (info->users > 0) - xenbus_dev_error(dev, -EBUSY, - "Device in use; refusing to close"); - else { - xlvbd_release_gendisk(info); - xenbus_frontend_closed(info->xbdev); - } - - mutex_unlock(&bd->bd_mutex); - bdput(bd); + blkfront_closing(info); break; } }