From nobody Sun Apr 28 08:28:13 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; dkim=fail; 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 1503311813389526.6931846249389; Mon, 21 Aug 2017 03:36:53 -0700 (PDT) Received: from localhost ([::1]:60356 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1djk4e-00011Z-RW for importer@patchew.org; Mon, 21 Aug 2017 06:36:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58566) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1djk3b-0000cO-KQ for qemu-devel@nongnu.org; Mon, 21 Aug 2017 06:35:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1djk3Y-00047B-CD for qemu-devel@nongnu.org; Mon, 21 Aug 2017 06:35:43 -0400 Received: from ozlabs.org ([103.22.144.67]:35565) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1djk3X-00040Y-O9; Mon, 21 Aug 2017 06:35:40 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3xbVT30lBDz9sNn; Mon, 21 Aug 2017 20:35:35 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1503311735; bh=BJOFcTTwJRWIFaXmzibsN2HhCOaQVpzTni5MRZ1oG9E=; h=From:To:Cc:Subject:Date:From; b=A94Xm4MV0wBUnzhDlFpNS7KybmU2+Z2/ynaFLIGCxOHDioHWOafHvOekjlBCv/RqK xSGT3C2oEEajNG0dBWV4ZkZU0aH54DAnsMY1HwyMUfqLv6vDFqajmNAo5VsLEcpSgU l/FiHlvIxbJmRM5APk62WOpDFMXD3SPWZcW43MHA= From: David Gibson To: cohuck@redhat.com, lvivier@redhat.com Date: Mon, 21 Aug 2017 20:35:24 +1000 Message-Id: <20170821103524.22619-1-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.13.5 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 103.22.144.67 Subject: [Qemu-devel] [PATCH] 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: qemu-ppc@nongnu.org, thuth@redhat.com, qemu-devel@nongnu.org, David Gibson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Thomas Huth 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 Reviewed-by: Igor Mammedov Signed-off-by: David Gibson Reviewed-by: Laurent Vivier --- 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 59435390ba..21081041d5 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 db896b0bb6..952fce5ec8 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 ea67b461c2..bdf6649083 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 f7a19720dc..cec441cbf4 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 1e483f2670..6f8c3eb1b3 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 2.13.5