From nobody Thu May 2 02:07:58 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1503297563388214.33581218040342; Sun, 20 Aug 2017 23:39:23 -0700 (PDT) Received: from localhost ([::1]:48715 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1djgMq-0003AG-P3 for importer@patchew.org; Mon, 21 Aug 2017 02:39:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56173) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1djgLw-0002mu-2a for qemu-devel@nongnu.org; Mon, 21 Aug 2017 02:38:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1djgLs-0000U5-VP for qemu-devel@nongnu.org; Mon, 21 Aug 2017 02:38:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38680) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1djgLs-0000NB-MP; Mon, 21 Aug 2017 02:38:20 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C7E9637EE9; Mon, 21 Aug 2017 06:30:34 +0000 (UTC) Received: from thh440s.redhat.com (ovpn-116-51.ams2.redhat.com [10.36.116.51]) by smtp.corp.redhat.com (Postfix) with ESMTP id 730CA600C8; Mon, 21 Aug 2017 06:30:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C7E9637EE9 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=thuth@redhat.com From: Thomas Huth To: qemu-devel@nongnu.org, Paolo Bonzini , Eduardo Habkost , David Gibson Date: Mon, 21 Aug 2017 08:30:29 +0200 Message-Id: <1503297029-28436-1-git-send-email-thuth@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 21 Aug 2017 06:30:34 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2] hw/ppc/spapr: Fix segfault when instantiating a 'pc-dimm' without 'memdev' 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: , Cc: Xiao Guangrong , Igor Mammedov , qemu-ppc@nongnu.org, "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" QEMU currently crashes when trying to use a 'pc-dimm' on the pseries machine without specifying its 'memdev' property. This happens because pc_dimm_get_memory_region() does not check whether the 'memdev' property has properly been set by the user. Looking closer at this function, it's also obvious that it is using &error_abort to call another function - and this is bad in a function that is used in the hot-plugging calling chain since this can also cause QEMU to exit unexpectedly. So let's fix these issues in a proper way now: Add a "Error **errp" parameter to pc_dimm_get_memory_region() which we use in case the 'memdev' property has not been set by the user, and which we can use instead of the &error_abort, and change the callers of get_memory_region() to make use of this "errp" parameter for proper error checking. Signed-off-by: Thomas Huth Acked-by: David Gibson Reviewed-by: David Gibson Reviewed-by: Igor Mammedov --- hw/i386/pc.c | 14 ++++++++++++-- hw/mem/nvdimm.c | 2 +- hw/mem/pc-dimm.c | 14 +++++++++++--- hw/ppc/spapr.c | 42 ++++++++++++++++++++++++++++++------------ include/hw/mem/pc-dimm.h | 2 +- 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 5943539..2108104 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1691,10 +1691,15 @@ static void pc_dimm_plug(HotplugHandler *hotplug_de= v, PCMachineClass *pcmc =3D PC_MACHINE_GET_CLASS(pcms); PCDIMMDevice *dimm =3D PC_DIMM(dev); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); + MemoryRegion *mr; uint64_t align =3D TARGET_PAGE_SIZE; bool is_nvdimm =3D object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); =20 + mr =3D ddc->get_memory_region(dimm, &local_err); + if (local_err) { + goto out; + } + if (memory_region_get_alignment(mr) && pcmc->enforce_aligned_dimm) { align =3D memory_region_get_alignment(mr); } @@ -1758,10 +1763,15 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_= dev, PCMachineState *pcms =3D PC_MACHINE(hotplug_dev); PCDIMMDevice *dimm =3D PC_DIMM(dev); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); + MemoryRegion *mr; HotplugHandlerClass *hhc; Error *local_err =3D NULL; =20 + mr =3D ddc->get_memory_region(dimm, &local_err); + if (local_err) { + goto out; + } + hhc =3D HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); =20 diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index db896b0..952fce5 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -71,7 +71,7 @@ static void nvdimm_init(Object *obj) NULL, NULL); } =20 -static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) +static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **= errp) { NVDIMMDevice *nvdimm =3D NVDIMM(dimm); =20 diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index ea67b46..bdf6649 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -363,7 +363,10 @@ static void pc_dimm_get_size(Object *obj, Visitor *v, = const char *name, PCDIMMDevice *dimm =3D PC_DIMM(obj); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(obj); =20 - mr =3D ddc->get_memory_region(dimm); + mr =3D ddc->get_memory_region(dimm, errp); + if (!mr) { + return; + } value =3D memory_region_size(mr); =20 visit_type_uint64(v, name, &value, errp); @@ -411,9 +414,14 @@ static void pc_dimm_unrealize(DeviceState *dev, Error = **errp) host_memory_backend_set_mapped(dimm->hostmem, false); } =20 -static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm) +static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error *= *errp) { - return host_memory_backend_get_memory(dimm->hostmem, &error_abort); + if (!dimm->hostmem) { + error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property must be set"); + return NULL; + } + + return host_memory_backend_get_memory(dimm->hostmem, errp); } =20 static MemoryRegion *pc_dimm_get_vmstate_memory_region(PCDIMMDevice *dimm) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f7a1972..cec441c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2772,10 +2772,15 @@ static void spapr_memory_plug(HotplugHandler *hotpl= ug_dev, DeviceState *dev, sPAPRMachineState *ms =3D SPAPR_MACHINE(hotplug_dev); PCDIMMDevice *dimm =3D PC_DIMM(dev); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); - uint64_t align =3D memory_region_get_alignment(mr); - uint64_t size =3D memory_region_size(mr); - uint64_t addr; + MemoryRegion *mr; + uint64_t align, size, addr; + + mr =3D ddc->get_memory_region(dimm, &local_err); + if (local_err) { + goto out; + } + align =3D memory_region_get_alignment(mr); + size =3D memory_region_size(mr); =20 pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, &local_err); if (local_err) { @@ -2808,10 +2813,16 @@ static void spapr_memory_pre_plug(HotplugHandler *h= otplug_dev, DeviceState *dev, { PCDIMMDevice *dimm =3D PC_DIMM(dev); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); - uint64_t size =3D memory_region_size(mr); + MemoryRegion *mr; + uint64_t size; char *mem_dev; =20 + mr =3D ddc->get_memory_region(dimm, errp); + if (!mr) { + return; + } + size =3D memory_region_size(mr); + if (size % SPAPR_MEMORY_BLOCK_SIZE) { error_setg(errp, "Hotplugged memory size must be a multiple of " "%lld MB", SPAPR_MEMORY_BLOCK_SIZE / M_BYTE); @@ -2882,7 +2893,7 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_sta= te(sPAPRMachineState *ms, { sPAPRDRConnector *drc; PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); + MemoryRegion *mr =3D ddc->get_memory_region(dimm, &error_abort); uint64_t size =3D memory_region_size(mr); uint32_t nr_lmbs =3D size / SPAPR_MEMORY_BLOCK_SIZE; uint32_t avail_lmbs =3D 0; @@ -2912,7 +2923,7 @@ void spapr_lmb_release(DeviceState *dev) sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_hotplug_handler(de= v)); PCDIMMDevice *dimm =3D PC_DIMM(dev); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); + MemoryRegion *mr =3D ddc->get_memory_region(dimm, &error_abort); sPAPRDIMMState *ds =3D spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(= dev)); =20 /* This information will get lost if a migration occurs @@ -2945,12 +2956,19 @@ static void spapr_memory_unplug_request(HotplugHand= ler *hotplug_dev, Error *local_err =3D NULL; PCDIMMDevice *dimm =3D PC_DIMM(dev); PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr =3D ddc->get_memory_region(dimm); - uint64_t size =3D memory_region_size(mr); - uint32_t nr_lmbs =3D size / SPAPR_MEMORY_BLOCK_SIZE; - uint64_t addr_start, addr; + MemoryRegion *mr; + uint32_t nr_lmbs; + uint64_t size, addr_start, addr; int i; sPAPRDRConnector *drc; + + mr =3D ddc->get_memory_region(dimm, &local_err); + if (local_err) { + goto out; + } + size =3D memory_region_size(mr); + nr_lmbs =3D size / SPAPR_MEMORY_BLOCK_SIZE; + addr_start =3D object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PRO= P, &local_err); if (local_err) { diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index 1e483f2..6f8c3eb 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -71,7 +71,7 @@ typedef struct PCDIMMDeviceClass { =20 /* public */ void (*realize)(PCDIMMDevice *dimm, Error **errp); - MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm); + MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm, Error **errp); MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm); } PCDIMMDeviceClass; =20 --=20 1.8.3.1