From nobody Mon Apr 29 10:35:52 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1493365583487454.85900711132683; Fri, 28 Apr 2017 00:46:23 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8CFE1C03BD5B; Fri, 28 Apr 2017 07:46:21 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C6D72171D1; Fri, 28 Apr 2017 07:46:20 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 0FC5B18521C2; Fri, 28 Apr 2017 07:46:20 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v3S7kH21010503 for ; Fri, 28 Apr 2017 03:46:17 -0400 Received: by smtp.corp.redhat.com (Postfix) id 0D6D960BE2; Fri, 28 Apr 2017 07:46:17 +0000 (UTC) Received: from beluga.usersys.redhat.com.com (dhcp129-94.brq.redhat.com [10.34.129.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3511D78344; Fri, 28 Apr 2017 07:46:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8CFE1C03BD5B Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 8CFE1C03BD5B From: Erik Skultety To: libvir-list@redhat.com Date: Fri, 28 Apr 2017 09:46:32 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Cc: Erik Skultety Subject: [libvirt] [PATCH 1/2] util: mdev: Use a local variable instead of an invalid pointer reference X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 28 Apr 2017 07:46:22 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" virMediatedDeviceListAdd calls VIR_APPEND_ELEMENT which normally clears the element to be added, thus the pointer must not be used afterwards, but because the pointer here is passed by value, what gets cleared is a copy of the original pointer that got created on the stack automatically when calling the function. The fact that it works now is just a coincidence and a bug in virMediatedDeviceListAdd that needs to be fixed in a separate commit. Therefore, use a local variable to hold data, rather than accessing the pointer later. Signed-off-by: Erik Skultety Reviewed-by: Laine Stump --- src/util/virmdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 2a1ade739..c1499d238 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -447,20 +447,21 @@ virMediatedDeviceListMarkDevices(virMediatedDeviceLis= tPtr dst, virObjectLock(dst); for (i =3D 0; i < count; i++) { virMediatedDevicePtr mdev =3D virMediatedDeviceListGet(src, i); + const char *mdev_path =3D mdev->path; =20 if (virMediatedDeviceIsUsed(mdev, dst) || virMediatedDeviceSetUsedBy(mdev, drvname, domname) < 0) goto cleanup; =20 /* Copy mdev references to the driver list: - * - caller is responsible for NOT freeing devices in @list on suc= cess + * - caller is responsible for NOT freeing devices in @src on succ= ess * - we're responsible for performing a rollback on failure */ if (virMediatedDeviceListAdd(dst, mdev) < 0) goto rollback; =20 VIR_DEBUG("'%s' added to list of active mediated devices used by '= %s'", - mdev->path, domname); + mdev_path, domname); } =20 ret =3D 0; --=20 2.12.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 10:35:52 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1493365583156215.31785478669133; Fri, 28 Apr 2017 00:46:23 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8975E3DBF2; Fri, 28 Apr 2017 07:46:21 +0000 (UTC) Received: from colo-mx.corp.redhat.com (unknown [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C6B858ACEF; Fri, 28 Apr 2017 07:46:20 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 662AE5EC62; Fri, 28 Apr 2017 07:46:19 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v3S7kILZ010510 for ; Fri, 28 Apr 2017 03:46:18 -0400 Received: by smtp.corp.redhat.com (Postfix) id 16D9778349; Fri, 28 Apr 2017 07:46:18 +0000 (UTC) Received: from beluga.usersys.redhat.com.com (dhcp129-94.brq.redhat.com [10.34.129.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 629F960BE2; Fri, 28 Apr 2017 07:46:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8975E3DBF2 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 8975E3DBF2 From: Erik Skultety To: libvir-list@redhat.com Date: Fri, 28 Apr 2017 09:46:33 +0200 Message-Id: <412ae229331a91dd6ba517e5165404ff4f585c8c.1493365484.git.eskultet@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Cc: Erik Skultety Subject: [libvirt] [PATCH 2/2] mdev: Fix daemon crash on domain shutdown after reconnect X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 28 Apr 2017 07:46:22 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The problem resides in virHostdevUpdateActiveMediatedDevices which gets called during qemuProcessReconnect. The issue here is that virMediatedDeviceListAdd takes a pointer to the item to be added to the list to which VIR_APPEND_ELEMENT is used, which also clears the pointer. However, in this case only the local copy of the pointer got cleared, leaving the original pointing to valid memory. To sum it up, during cleanup phase, the original pointer is freed and the daemon crashes basically any time it would access it. Backtrace: 0x00007ffff3ccdeba in __strcmp_sse2_unaligned 0x00007ffff72a444a in virMediatedDeviceListFindIndex 0x00007ffff7241446 in virHostdevReAttachMediatedDevices 0x00007fffc60215d9 in qemuHostdevReAttachMediatedDevices 0x00007fffc60216dc in qemuHostdevReAttachDomainDevices 0x00007fffc6046e6f in qemuProcessStop 0x00007fffc6091596 in processMonitorEOFEvent 0x00007fffc6091793 in qemuProcessEventHandler 0x00007ffff7294bf5 in virThreadPoolWorker 0x00007ffff7294184 in virThreadHelper 0x00007ffff3fdc3c4 in start_thread () from /lib64/libpthread.so.0 0x00007ffff3d269cf in clone () from /lib64/libc.so.6 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=3D1446455 Signed-off-by: Erik Skultety Reviewed-by: Laine Stump --- src/util/virhostdev.c | 4 ++-- src/util/virmdev.c | 13 ++++++++----- src/util/virmdev.h | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 2c557f5bb..579563c3f 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1294,7 +1294,7 @@ virHostdevUpdateActiveMediatedDevices(virHostdevManag= erPtr mgr, =20 virMediatedDeviceSetUsedBy(mdev, drv_name, dom_name); =20 - if (virMediatedDeviceListAdd(mgr->activeMediatedHostdevs, mdev) < = 0) + if (virMediatedDeviceListAdd(mgr->activeMediatedHostdevs, &mdev) <= 0) goto cleanup; } =20 @@ -1790,7 +1790,7 @@ virHostdevPrepareMediatedDevices(virHostdevManagerPtr= mgr, if (!(mdev =3D virMediatedDeviceNew(src->uuidstr, src->model))) goto cleanup; =20 - if (virMediatedDeviceListAdd(list, mdev) < 0) { + if (virMediatedDeviceListAdd(list, &mdev) < 0) { virMediatedDeviceFree(mdev); goto cleanup; } diff --git a/src/util/virmdev.c b/src/util/virmdev.c index c1499d238..55ec8db9f 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -312,16 +312,19 @@ virMediatedDeviceListDispose(void *obj) } =20 =20 +/* The reason for @dev to be double pointer is that VIR_APPEND_ELEMENT cle= ars + * the pointer and we need to clear the original no a copy on the stack + */ int virMediatedDeviceListAdd(virMediatedDeviceListPtr list, - virMediatedDevicePtr dev) + virMediatedDevicePtr *dev) { - if (virMediatedDeviceListFind(list, dev)) { + if (virMediatedDeviceListFind(list, *dev)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("device %s is already in use"), dev->path); + _("device %s is already in use"), (*dev)->path); return -1; } - return VIR_APPEND_ELEMENT(list->devs, list->count, dev); + return VIR_APPEND_ELEMENT(list->devs, list->count, *dev); } =20 =20 @@ -457,7 +460,7 @@ virMediatedDeviceListMarkDevices(virMediatedDeviceListP= tr dst, * - caller is responsible for NOT freeing devices in @src on succ= ess * - we're responsible for performing a rollback on failure */ - if (virMediatedDeviceListAdd(dst, mdev) < 0) + if (virMediatedDeviceListAdd(dst, &mdev) < 0) goto rollback; =20 VIR_DEBUG("'%s' added to list of active mediated devices used by '= %s'", diff --git a/src/util/virmdev.h b/src/util/virmdev.h index 2f3d6bb84..8bb46b9c5 100644 --- a/src/util/virmdev.h +++ b/src/util/virmdev.h @@ -86,7 +86,7 @@ virMediatedDeviceListNew(void); =20 int virMediatedDeviceListAdd(virMediatedDeviceListPtr list, - virMediatedDevicePtr dev); + virMediatedDevicePtr *dev); =20 virMediatedDevicePtr virMediatedDeviceListGet(virMediatedDeviceListPtr list, --=20 2.12.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list