From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=45761 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q5Coz-0002Oo-Q1
	for qemu-devel@nongnu.org; Thu, 31 Mar 2011 04:01:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Q5Cou-0008Hb-Gn
	for qemu-devel@nongnu.org; Thu, 31 Mar 2011 04:01:35 -0400
Received: from mx1.redhat.com ([209.132.183.28]:12231)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Q5Cou-0008H6-7g
	for qemu-devel@nongnu.org; Thu, 31 Mar 2011 04:01:32 -0400
Message-ID: <4D943559.5060305@redhat.com>
Date: Thu, 31 Mar 2011 10:03:37 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <20110330015147.GG5445@us.ibm.com>
In-Reply-To: <20110330015147.GG5445@us.ibm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH v4] Do not delete BlockDriverState when
	deleting the drive
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Ryan Harper <ryanh@us.ibm.com>
Cc: Stefan Hajnoczi <stefan.hajnoczi@uk.ibm.com>, Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org

Am 30.03.2011 03:51, schrieb Ryan Harper:
> When removing a drive from the host-side via drive_del we currently have
> the following path:
> 
> drive_del
> qemu_aio_flush()
> bdrv_close()    // zaps bs->drv, which makes any subsequent I/O get
>                 // dropped.  Works as designed
> drive_uninit()
> bdrv_delete()   // frees the bs.  Since the device is still connected to
>                 // bs, any subsequent I/O is a use-after-free.
> 
> The value of bs->drv becomes unpredictable on free.  As long as it
> remains null, I/O still gets dropped, however it could become non-null
> at any point after the free resulting SEGVs or other QEMU state
> corruption.
> 
> 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 its own function, bdrv_make_anon().
> 
> 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.
> 
> We also don't attempt to remove the qdev property since we are no longer
> deleting the BlockDriverState on drives with associated drives.  This
> also allows for removing Drives with no devices associated either.
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

Thanks, applied to the block branch.

Kevin