From patchwork Wed Aug 6 20:39:26 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 377603 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B61E514009C for ; Thu, 7 Aug 2014 07:19:06 +1000 (EST) Received: from localhost ([::1]:41353 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF8bw-0000Zm-Pj for incoming@patchwork.ozlabs.org; Wed, 06 Aug 2014 17:19:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38949) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF83v-0001Tf-KD for qemu-devel@nongnu.org; Wed, 06 Aug 2014 16:44:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XF83Y-0005Z6-0H for qemu-devel@nongnu.org; Wed, 06 Aug 2014 16:43:55 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:34327) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF83X-0005Yg-Qe for qemu-devel@nongnu.org; Wed, 06 Aug 2014 16:43:31 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 6 Aug 2014 16:43:31 -0400 Received: from d01dlp03.pok.ibm.com (9.56.250.168) by e7.ny.us.ibm.com (192.168.1.107) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 6 Aug 2014 16:43:28 -0400 Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id A3321C9003E; Wed, 6 Aug 2014 16:43:20 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by b01cxnp22033.gho.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s76KhSkb5833088; Wed, 6 Aug 2014 20:43:28 GMT Received: from d01av01.pok.ibm.com (localhost [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s76KhRD2025562; Wed, 6 Aug 2014 16:43:27 -0400 Received: from localhost ([9.80.101.111]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s76KhR4D025553; Wed, 6 Aug 2014 16:43:27 -0400 From: Michael Roth To: qemu-devel@nongnu.org Date: Wed, 6 Aug 2014 15:39:26 -0500 Message-Id: <1407357598-21541-77-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1407357598-21541-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1407357598-21541-1-git-send-email-mdroth@linux.vnet.ibm.com> MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14080620-5806-0000-0000-00000028FF44 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 32.97.182.137 Cc: qemu-stable@nongnu.org Subject: [Qemu-devel] [PATCH 076/108] qdev: recursively unrealize devices when unrealizing bus X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Paolo Bonzini When the patch was posted that became 5c21ce7 (qdev: Realize buses on device realization, 2014-03-12), it included recursive realization and unrealization of devices when the bus's "realized" property was toggled. However, due to the same old worries about recursive realization and prerequisites not being realized yet, those hunks were dropped when committing the patch. Unfortunately, this causes a use-after-free bug (easily reproduced by a PCI hot-unplug action). Before the patch, device_unparent behaved as follows: for each child bus unparent bus ----------------------------. | for each child device | | unparent device ---------------. | | | unrealize device | | | | call dc->unparent | | | '------------------------------- | '----------------------------------------' unrealize device After the patch, it behaves as follows instead: unrealize device --------------------. | for each child bus | | unrealize bus (A) | '------------------------------------' for each child bus unparent bus ----------------------. | for each child device | | unrealize device (B) | | call dc->unparent | '----------------------------------' At the step marked (B) the device might use data from the bus that is not available anymore due to step (A). To fix this, we need to unrealize devices before step (A). To sidestep concerns about recursive realization, only do recursive unrealization and leave the "value && !bus->realized" case as it is. The resulting flow is: for each child bus unrealize bus ---------------------. | for each child device | | unrealize device (B) | | call bc->unrealize (A) | '----------------------------------' unrealize device for each child bus unparent bus ----------------------. | for each child device | | unparent device | '----------------------------------' where everything is "powered down" before it is unassembled. Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini Tested-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Andreas Färber (cherry picked from commit 5942a19040fed313b316ab7b6e3d2d8e7b1625bb) Signed-off-by: Michael Roth --- hw/core/qdev.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index f52f0ac..79db470 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -515,14 +515,25 @@ static void bus_set_realized(Object *obj, bool value, Error **errp) { BusState *bus = BUS(obj); BusClass *bc = BUS_GET_CLASS(bus); + BusChild *kid; Error *local_err = NULL; if (value && !bus->realized) { if (bc->realize) { bc->realize(bus, &local_err); } + + /* TODO: recursive realization */ } else if (!value && bus->realized) { - if (bc->unrealize) { + QTAILQ_FOREACH(kid, &bus->children, sibling) { + DeviceState *dev = kid->child; + object_property_set_bool(OBJECT(dev), false, "realized", + &local_err); + if (local_err != NULL) { + break; + } + } + if (bc->unrealize && local_err == NULL) { bc->unrealize(bus, &local_err); } }