From patchwork Wed Mar 2 15:03:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ryan Harper X-Patchwork-Id: 85092 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4C565B7102 for ; Thu, 3 Mar 2011 02:07:25 +1100 (EST) Received: from localhost ([127.0.0.1]:39087 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pune2-00064Z-HM for incoming@patchwork.ozlabs.org; Wed, 02 Mar 2011 10:07:18 -0500 Received: from [140.186.70.92] (port=40529 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Punai-0004pG-34 for qemu-devel@nongnu.org; Wed, 02 Mar 2011 10:03:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Punag-0004rm-Gw for qemu-devel@nongnu.org; Wed, 02 Mar 2011 10:03:51 -0500 Received: from e38.co.us.ibm.com ([32.97.110.159]:43749) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Punag-0004rE-BN for qemu-devel@nongnu.org; Wed, 02 Mar 2011 10:03:50 -0500 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e38.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p227nGoO011236 for ; Wed, 2 Mar 2011 00:49:16 -0700 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id p22F3lXW115914 for ; Wed, 2 Mar 2011 08:03:47 -0700 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p22F3l6v004439 for ; Wed, 2 Mar 2011 08:03:47 -0700 Received: from localhost.localdomain (frylock.austin.ibm.com [9.53.41.12]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p22F3lfv004426; Wed, 2 Mar 2011 08:03:47 -0700 Received: by localhost.localdomain (Postfix, from userid 1000) id 6AACB1BF81; Wed, 2 Mar 2011 09:03:42 -0600 (CST) Date: Wed, 2 Mar 2011 09:03:42 -0600 From: Ryan Harper To: Markus Armbruster Message-ID: <20110302150342.GZ13257@us.ibm.com> Mime-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.6+20040907i X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 32.97.110.159 Cc: qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH] Do not delete BlockDriverState when deleting the drive X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org When removing a drive from the host-side via drive_del we currently have the following path: drive_del qemu_aio_flush() bdrv_close() drive_uninit() bdrv_delete() When we bdrv_delete() we end up qemu_free()'ing the BlockDriverState pointer however, the block devices retain a copy of this pointer, see hw/virtio-blk.c:virtio_blk_init() where we s->bs = conf->bs. We now have a use-after-free situation. If the guest continues to issue IO against the device, and we've reallocated the memory that the BlockDriverState pointed at, then we will fail the bs->drv checks in the various bdrv_ methods. To resolve this issue as simply as possible, we can chose to not actually delete the BlockDriverState pointer. Since bdrv_close() handles setting the drv pointer to NULL, we just need to remove the BlockDriverState from the QLIST that is used to enumerate the block devices. This is currently handled within bdrv_delete, so move this into it's own function, bdrv_remove(). The result is that we can now invoke drive_del, this closes the file descriptors and sets BlockDriverState->drv to NULL which prevents futher IO to the device, and since we do not free BlockDriverState, we don't have to worry about the copy retained in the block devices. This state will be deleted if the guest is asked and responds to a pci device removal request. Reported-by: Markus Armbruster Signed-off-by: Ryan Harper --- block.c | 11 ++++++++--- block.h | 1 + blockdev.c | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index f7d91a2..92dd3fe 100644 --- a/block.c +++ b/block.c @@ -697,14 +697,19 @@ void bdrv_close_all(void) } } +void bdrv_remove(BlockDriverState *bs) +{ + if (bs->device_name[0] != '\0') { + QTAILQ_REMOVE(&bdrv_states, bs, list); + } +} + void bdrv_delete(BlockDriverState *bs) { assert(!bs->peer); /* remove from list, if necessary */ - if (bs->device_name[0] != '\0') { - QTAILQ_REMOVE(&bdrv_states, bs, list); - } + bdrv_remove(bs); bdrv_close(bs); if (bs->file != NULL) { diff --git a/block.h b/block.h index 5d78fc0..8447397 100644 --- a/block.h +++ b/block.h @@ -66,6 +66,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, QEMUOptionParameter *options); int bdrv_create_file(const char* filename, QEMUOptionParameter *options); BlockDriverState *bdrv_new(const char *device_name); +void bdrv_remove(BlockDriverState *bs); void bdrv_delete(BlockDriverState *bs); int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags); int bdrv_open(BlockDriverState *bs, const char *filename, int flags, diff --git a/blockdev.c b/blockdev.c index 0690cc8..1f57b50 100644 --- a/blockdev.c +++ b/blockdev.c @@ -760,7 +760,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) } /* clean up host side */ - drive_uninit(drive_get_by_blockdev(bs)); + bdrv_remove(bs); return 0; }