From patchwork Fri Jun 16 01:37:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bharata B Rao X-Patchwork-Id: 776530 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 3wpjh056DLz9rvt for ; Fri, 16 Jun 2017 11:38:40 +1000 (AEST) Received: from localhost ([::1]:56772 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLgDd-0006SX-2N for incoming@patchwork.ozlabs.org; Thu, 15 Jun 2017 21:38:37 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44524) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLgDA-0006On-Q4 for qemu-devel@nongnu.org; Thu, 15 Jun 2017 21:38:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLgD7-0006hn-KQ for qemu-devel@nongnu.org; Thu, 15 Jun 2017 21:38:08 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:49053 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dLgD7-0006hM-EH for qemu-devel@nongnu.org; Thu, 15 Jun 2017 21:38:05 -0400 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v5G1YFEQ136343 for ; Thu, 15 Jun 2017 21:38:04 -0400 Received: from e23smtp09.au.ibm.com (e23smtp09.au.ibm.com [202.81.31.142]) by mx0b-001b2d01.pphosted.com with ESMTP id 2b42v35gwt-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 15 Jun 2017 21:38:04 -0400 Received: from localhost by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 16 Jun 2017 11:38:01 +1000 Received: from d23relay09.au.ibm.com (202.81.31.228) by e23smtp09.au.ibm.com (202.81.31.206) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 16 Jun 2017 11:37:58 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay09.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v5G1bvfp62259426; Fri, 16 Jun 2017 11:37:57 +1000 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id v5G1btKx007685; Fri, 16 Jun 2017 11:37:55 +1000 Received: from in.ibm.com ([9.77.208.227]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id v5G1bpVI007609 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 16 Jun 2017 11:37:54 +1000 Date: Fri, 16 Jun 2017 07:07:53 +0530 From: Bharata B Rao To: Greg Kurz References: <1497495164-9894-1-git-send-email-bharata@linux.vnet.ibm.com> <20170615093238.65532c77@bahia.ttt.fr.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170615093238.65532c77@bahia.ttt.fr.ibm.com> User-Agent: Mutt/1.7.1 (2016-10-04) X-TM-AS-MML: disable x-cbid: 17061601-0052-0000-0000-000002573AE5 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17061601-0053-0000-0000-000008359204 Message-Id: <20170616013753.GA9658@in.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-06-16_01:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1706160022 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 148.163.158.5 Subject: Re: [Qemu-devel] [FIX PATCH] spapr: prevent QEMU crash when CPU realization fails X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: bharata@linux.vnet.ibm.com Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, david@gibson.dropbear.id.au Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On Thu, Jun 15, 2017 at 09:32:38AM +0200, Greg Kurz wrote: > On Thu, 15 Jun 2017 08:22:44 +0530 > Bharata B Rao wrote: > > > ICPState objects were being allocated before CPU thread realization. > > However commit 9ed656631d73 (xics: setup cpu at realize time) reversed it > > by allocating ICPState objects after CPU thread is realized. But it > > didn't take care to fix the error path because of which we observe > > a SIGSEGV when CPU thread realization fails during cold/hotplug. > > > > Fix this by ensuring that we do object_unparent() of ICPState object > > only in case when is was created earlier. > > > > Oops, my bad... my initial intent was to conditionally call object_unparent() > and I simply forgot to put the "if (obj) { }". But your patch is valid as well > of course. Maybe you can drop the initialization of obj to NULL on the way, > since it really doesn't make sense anymore. Here is the version w/o initializing obj to NULL. From cb9cc946df0d1c430cccb1463d78fa4b41e9f0ee Mon Sep 17 00:00:00 2001 From: Bharata B Rao Date: Wed, 14 Jun 2017 19:24:43 +0530 Subject: [FIX PATCH v1] spapr: prevent QEMU crash when CPU realization fails ICPState objects were being allocated before CPU thread realization. However commit 9ed656631d73 (xics: setup cpu at realize time) reversed it by allocating ICPState objects after CPU thread is realized. But it didn't take care to fix the error path because of which we observe a SIGSEGV when CPU thread realization fails during cold/hotplug. Fix this by ensuring that we do object_unparent() of ICPState object only in case when is was created earlier. Signed-off-by: Bharata B Rao Reviewed-by: Greg Kurz --- hw/ppc/spapr_cpu_core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index d6719d5..ea278ce 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -178,7 +178,7 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp) sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); CPUState *cs = CPU(child); PowerPCCPU *cpu = POWERPC_CPU(cs); - Object *obj = NULL; + Object *obj; object_property_set_bool(child, true, "realized", &local_err); if (local_err) { @@ -198,13 +198,14 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp) object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); object_property_set_bool(obj, true, "realized", &local_err); if (local_err) { - goto error; + goto free_icp; } return; -error: +free_icp: object_unparent(obj); +error: error_propagate(errp, local_err); }