From nobody Fri May 3 23:48:39 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.zohomail.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 1500988930959592.7276226998339; Tue, 25 Jul 2017 06:22:10 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 189B613A45; Tue, 25 Jul 2017 13:22:07 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E037860602; Tue, 25 Jul 2017 13:22:06 +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 727AE4BB79; Tue, 25 Jul 2017 13:22:06 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v6PDM57Y006243 for ; Tue, 25 Jul 2017 09:22:05 -0400 Received: by smtp.corp.redhat.com (Postfix) id 15A0B8E836; Tue, 25 Jul 2017 13:22:05 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-46.phx2.redhat.com [10.3.117.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id D15D78F343 for ; Tue, 25 Jul 2017 13:22:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 189B613A45 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=pass smtp.mailfrom=libvir-list-bounces@redhat.com From: John Ferlan To: libvir-list@redhat.com Date: Tue, 25 Jul 2017 09:21:53 -0400 Message-Id: <20170725132155.5150-2-jferlan@redhat.com> In-Reply-To: <20170725132155.5150-1-jferlan@redhat.com> References: <20170725132155.5150-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v3 1/3] secret: Properly handle @def after virSecretObjAdd in driver 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.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 25 Jul 2017 13:22:07 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Since the virSecretObjListAdd technically consumes @def on success, the secretDefineXML should set @def =3D NULL immediately and process the remaining calls using a new @objDef variable. We can use use VIR_STEAL_PTR since we know the Add function just stores @def in obj->def. Because we steal @def into @objDef, if we jump to restore_backup: and @backup is set, then we need to ensure the @def would be free'd properly, so we'll steal it back from @objDef. For the other condition this fixes a double free of @def if the code had jumped to @backup =3D=3D NULL thus calling virSecretObjListRemove without setting @def =3D NULL. In this case, the subsequent call to DefFree would succeed and free @def; however, the call to EndAPI would also call DefFree because the Unref done would be the last one for the @obj meaning the obj->def would be used to call DefFree, but it's already been free'd because @def wasn't managed right within this error path. Signed-off-by: John Ferlan --- src/secret/secret_driver.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 30124b4..8defa46 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -210,6 +210,7 @@ secretDefineXML(virConnectPtr conn, { virSecretPtr ret =3D NULL; virSecretObjPtr obj =3D NULL; + virSecretDefPtr objDef; virSecretDefPtr backup =3D NULL; virSecretDefPtr def; virObjectEventPtr event =3D NULL; @@ -225,8 +226,9 @@ secretDefineXML(virConnectPtr conn, if (!(obj =3D virSecretObjListAdd(driver->secrets, def, driver->configDir, &backup))) goto cleanup; + VIR_STEAL_PTR(objDef, def); =20 - if (!def->isephemeral) { + if (!objDef->isephemeral) { if (backup && backup->isephemeral) { if (virSecretObjSaveData(obj) < 0) goto restore_backup; @@ -248,28 +250,27 @@ secretDefineXML(virConnectPtr conn, /* Saved successfully - drop old values */ virSecretDefFree(backup); =20 - event =3D virSecretEventLifecycleNew(def->uuid, - def->usage_type, - def->usage_id, + event =3D virSecretEventLifecycleNew(objDef->uuid, + objDef->usage_type, + objDef->usage_id, VIR_SECRET_EVENT_DEFINED, 0); =20 ret =3D virGetSecret(conn, - def->uuid, - def->usage_type, - def->usage_id); - def =3D NULL; + objDef->uuid, + objDef->usage_type, + objDef->usage_id); goto cleanup; =20 restore_backup: /* If we have a backup, then secret was defined before, so just restore - * the backup. The current def will be handled below. - * Otherwise, this is a new secret, thus remove it. - */ - if (backup) + * the backup; otherwise, this is a new secret, thus remove it. */ + if (backup) { virSecretObjSetDef(obj, backup); - else + VIR_STEAL_PTR(def, objDef); + } else { virSecretObjListRemove(driver->secrets, obj); + } =20 cleanup: virSecretDefFree(def); --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri May 3 23:48:39 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.zohomail.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 1500988935028713.1794981666942; Tue, 25 Jul 2017 06:22:15 -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 EC843C0587E8; Tue, 25 Jul 2017 13:22:11 +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 C1EA9934D5; Tue, 25 Jul 2017 13:22:11 +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 45451180B467; Tue, 25 Jul 2017 13:22:11 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v6PDM5Da006255 for ; Tue, 25 Jul 2017 09:22:05 -0400 Received: by smtp.corp.redhat.com (Postfix) id 73B758F343; Tue, 25 Jul 2017 13:22:05 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-46.phx2.redhat.com [10.3.117.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id 370348E836 for ; Tue, 25 Jul 2017 13:22:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EC843C0587E8 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com From: John Ferlan To: libvir-list@redhat.com Date: Tue, 25 Jul 2017 09:21:54 -0400 Message-Id: <20170725132155.5150-3-jferlan@redhat.com> In-Reply-To: <20170725132155.5150-1-jferlan@redhat.com> References: <20170725132155.5150-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v3 2/3] secret: Fix memory leak in virSecretLoad 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.32]); Tue, 25 Jul 2017 13:22:12 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" If the virSecretLoadValue fails, the code jumped to cleanup without setting @ret =3D obj, thus calling virSecretObjListRemove which only accounts for the object reference related to adding the object to the list during virSecretObjListAdd, but does not account for the reference to the object itself as the return of @ret would be NULL so the caller wouldn't call virSecretObjEndAPI on the object recently added thus reducing the refcnt to zero. This patch will perform the ObjListRemove in the failure path of virSecretLoadValue and Unref @obj in order to perform clean up and return @obj as NULL. The @def will be freed as part of the virObjectUnref. Signed-off-by: John Ferlan --- src/conf/virsecretobj.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 850fdaf..aa994ba 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -914,7 +914,6 @@ virSecretLoad(virSecretObjListPtr secrets, { virSecretDefPtr def =3D NULL; virSecretObjPtr obj =3D NULL; - virSecretObjPtr ret =3D NULL; =20 if (!(def =3D virSecretDefParseFile(path))) goto cleanup; @@ -926,16 +925,15 @@ virSecretLoad(virSecretObjListPtr secrets, goto cleanup; def =3D NULL; =20 - if (virSecretLoadValue(obj) < 0) - goto cleanup; - - ret =3D obj; - obj =3D NULL; + if (virSecretLoadValue(obj) < 0) { + virSecretObjListRemove(secrets, obj); + virObjectUnref(obj); + obj =3D NULL; + } =20 cleanup: - virSecretObjListRemove(secrets, obj); virSecretDefFree(def); - return ret; + return obj; } =20 =20 --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri May 3 23:48:39 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.zohomail.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 1500988933928784.9994548363887; Tue, 25 Jul 2017 06:22:13 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9423761D37; Tue, 25 Jul 2017 13:22:09 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6A17660602; Tue, 25 Jul 2017 13:22:09 +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 1E59E4A469; Tue, 25 Jul 2017 13:22:09 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v6PDM6aA006274 for ; Tue, 25 Jul 2017 09:22:06 -0400 Received: by smtp.corp.redhat.com (Postfix) id 1588C8F344; Tue, 25 Jul 2017 13:22:06 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-46.phx2.redhat.com [10.3.117.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id D2D768E836 for ; Tue, 25 Jul 2017 13:22:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9423761D37 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com From: John Ferlan To: libvir-list@redhat.com Date: Tue, 25 Jul 2017 09:21:55 -0400 Message-Id: <20170725132155.5150-4-jferlan@redhat.com> In-Reply-To: <20170725132155.5150-1-jferlan@redhat.com> References: <20170725132155.5150-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v3 3/3] secret: Handle object list removal and deletion properly 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.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 25 Jul 2017 13:22:10 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Rather than rely on virSecretObjEndAPI to make the final virObjectUnref after the call to virSecretObjListRemove, be more explicit by calling virObjectUnref and setting @obj to NULL for secretUndefine and in the error path of secretDefineXML. Calling EndAPI will end up calling Unlock on an already unlocked object which has indeteriminate results (usually an ignored error). The virSecretObjEndAPI will both Unref and Unlock the object; however, the virSecretObjListRemove would have already Unlock'd the object so calling Unlock again is incorrect. Once the virSecretObjListRemove is called all that's left is to Unref our interest since that's the corrollary to the virSecretObjListAdd which returned our ref interest plus references for each hash table in which the object resides. In math terms, after an Add there's 2 refs on the object (1 for the object and 1 for the list). After calling Remove there's just 1 ref on the object. For the Add callers, calling EndAPI removes the ref for the object and unlocks it, but since it's in a list the other 1 remains. Signed-off-by: John Ferlan --- src/secret/secret_driver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 8defa46..d833a86 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -270,6 +270,8 @@ secretDefineXML(virConnectPtr conn, VIR_STEAL_PTR(def, objDef); } else { virSecretObjListRemove(driver->secrets, obj); + virObjectUnref(obj); + obj =3D NULL; } =20 cleanup: @@ -410,6 +412,8 @@ secretUndefine(virSecretPtr secret) virSecretObjDeleteData(obj); =20 virSecretObjListRemove(driver->secrets, obj); + virObjectUnref(obj); + obj =3D NULL; =20 ret =3D 0; =20 --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list