From nobody Sun Apr 28 19:09:44 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 1501580525458718.843196291946; Tue, 1 Aug 2017 02:42:05 -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 42CDF622AB; Tue, 1 Aug 2017 09:42:03 +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 D898B67C9E; Tue, 1 Aug 2017 09:42:02 +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 263F74BB79; Tue, 1 Aug 2017 09:42:02 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v719fYhM007429 for ; Tue, 1 Aug 2017 05:41:34 -0400 Received: by smtp.corp.redhat.com (Postfix) id 7409366FE7; Tue, 1 Aug 2017 09:41:34 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id E20BD62943 for ; Tue, 1 Aug 2017 09:41:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 42CDF622AB Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 1 Aug 2017 11:41:25 +0200 Message-Id: <3e609bf4e483d85bf6e891fd7a23af4c3b291f27.1501580396.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/2] virCgroupValidateMachineGroup: Don't free @machinename 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.27]); Tue, 01 Aug 2017 09:42:04 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" We are given a string in @machinename, we never allocate it, just merely use it for reading. We should not free it otherwise it leads to double free: =3D=3D32191=3D=3D Thread 17: =3D=3D32191=3D=3D Invalid free() / delete / delete[] / realloc() =3D=3D32191=3D=3D at 0x4C2D1A0: free (vg_replace_malloc.c:530) =3D=3D32191=3D=3D by 0x54BBB84: virFree (viralloc.c:582) =3D=3D32191=3D=3D by 0x2BC04499: qemuProcessStop (qemu_process.c:6313) =3D=3D32191=3D=3D by 0x2BC500FF: processMonitorEOFEvent (qemu_driver.c:4= 724) =3D=3D32191=3D=3D by 0x2BC502FC: qemuProcessEventHandler (qemu_driver.c:= 4769) =3D=3D32191=3D=3D by 0x5550640: virThreadPoolWorker (virthreadpool.c:167) =3D=3D32191=3D=3D by 0x554FBCF: virThreadHelper (virthread.c:206) =3D=3D32191=3D=3D by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.= so) =3D=3D32191=3D=3D by 0x928DE3C: clone (in /lib64/libc-2.23.so) =3D=3D32191=3D=3D Address 0x31893d70 is 0 bytes inside a block of size 1,1= 00 free'd =3D=3D32191=3D=3D at 0x4C2D1A0: free (vg_replace_malloc.c:530) =3D=3D32191=3D=3D by 0x54BBB84: virFree (viralloc.c:582) =3D=3D32191=3D=3D by 0x54C1936: virCgroupValidateMachineGroup (vircgroup= .c:343) =3D=3D32191=3D=3D by 0x54C4B29: virCgroupNewDetectMachine (vircgroup.c:1= 550) =3D=3D32191=3D=3D by 0x2BBDDA29: qemuConnectCgroup (qemu_cgroup.c:972) =3D=3D32191=3D=3D by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:68= 22) =3D=3D32191=3D=3D by 0x554FBCF: virThreadHelper (virthread.c:206) =3D=3D32191=3D=3D by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.= so) =3D=3D32191=3D=3D by 0x928DE3C: clone (in /lib64/libc-2.23.so) =3D=3D32191=3D=3D Block was alloc'd at =3D=3D32191=3D=3D at 0x4C2BE80: malloc (vg_replace_malloc.c:298) =3D=3D32191=3D=3D by 0x4C2E35F: realloc (vg_replace_malloc.c:785) =3D=3D32191=3D=3D by 0x54BB492: virReallocN (viralloc.c:245) =3D=3D32191=3D=3D by 0x54BEDF2: virBufferGrow (virbuffer.c:150) =3D=3D32191=3D=3D by 0x54BF3B9: virBufferVasprintf (virbuffer.c:408) =3D=3D32191=3D=3D by 0x54BF324: virBufferAsprintf (virbuffer.c:381) =3D=3D32191=3D=3D by 0x55BB271: virDomainGenerateMachineName (domain_con= f.c:27078) =3D=3D32191=3D=3D by 0x2BBD5B8F: qemuDomainGetMachineName (qemu_domain.c= :9595) =3D=3D32191=3D=3D by 0x2BBDD9B4: qemuConnectCgroup (qemu_cgroup.c:966) =3D=3D32191=3D=3D by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:68= 22) =3D=3D32191=3D=3D by 0x554FBCF: virThreadHelper (virthread.c:206) =3D=3D32191=3D=3D by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.= so) Moreover, make the @machinename 'const char *' to mark it explicitly that we are not changing the passed string. Signed-off-by: Michal Privoznik --- src/util/vircgroup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index f274aee81..0a31947b0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -253,7 +253,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group, const char *name, const char *drivername, bool stripEmulatorSuffix, - char *machinename) + const char *machinename) { size_t i; bool valid =3D false; @@ -340,7 +340,6 @@ virCgroupValidateMachineGroup(virCgroupPtr group, VIR_FREE(partname); VIR_FREE(scopename_old); VIR_FREE(scopename_new); - VIR_FREE(machinename); return valid; } =20 --=20 2.13.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun Apr 28 19:09:44 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 1501580530871655.1853384835827; Tue, 1 Aug 2017 02:42:10 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4DC2CC02F96C; Tue, 1 Aug 2017 09:42:08 +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 183C5619B0; Tue, 1 Aug 2017 09:42:08 +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 B32AE180B467; Tue, 1 Aug 2017 09:42:07 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v719fZ2d007486 for ; Tue, 1 Aug 2017 05:41:35 -0400 Received: by smtp.corp.redhat.com (Postfix) id B261966FE2; Tue, 1 Aug 2017 09:41:35 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2D7C96294A for ; Tue, 1 Aug 2017 09:41:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4DC2CC02F96C 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=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 1 Aug 2017 11:41:26 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/2] virNetDaemonCallInhibit: Call virNetDaemonGotInhibitReply 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.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 01 Aug 2017 09:42:09 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" So there are couple of issues here. Firstly, we never unref the @pendingReply and thus it leaks. =3D=3D13279=3D=3D 144 (72 direct, 72 indirect) bytes in 1 blocks are defini= tely lost in loss record 1,095 of 1,259 =3D=3D13279=3D=3D at 0x4C2E080: calloc (vg_replace_malloc.c:711) =3D=3D13279=3D=3D by 0x781FA97: _dbus_pending_call_new_unlocked (in /usr= /lib64/libdbus-1.so.3.14.11) =3D=3D13279=3D=3D by 0x7812A4C: dbus_connection_send_with_reply (in /usr= /lib64/libdbus-1.so.3.14.11) =3D=3D13279=3D=3D by 0x56BEDF3: virNetDaemonCallInhibit (virnetdaemon.c:= 514) =3D=3D13279=3D=3D by 0x56BEF18: virNetDaemonAddShutdownInhibition (virne= tdaemon.c:536) =3D=3D13279=3D=3D by 0x12473B: daemonInhibitCallback (libvirtd.c:742) =3D=3D13279=3D=3D by 0x1249BD: daemonRunStateInit (libvirtd.c:823) =3D=3D13279=3D=3D by 0x554FBCF: virThreadHelper (virthread.c:206) =3D=3D13279=3D=3D by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.= so) =3D=3D13279=3D=3D by 0x928DE3C: clone (in /lib64/libc-2.23.so) Secondly, while we send the message, we are suspended ('cos we're talking to a UNIX socket). However, until we are resumed back again the reply might have came therefore subsequent dbus_pending_call_set_notify() has no effect and in fact the virNetDaemonGotInhibitReply() callback is never called. Thirdly, the dbus_connection_send_with_reply() has really stupid policy for return values. To cite the man page: Returns FALSE if no memory, TRUE otherwise. Yes, that's right. If anything goes wrong and it's not case of OOM then TRUE is returned, i.e. you're trying to pass FDs and it's not supported, or you're not connected, or anything else. Therefore, checking for return value of dbus_connection_send_with_reply() is not enoguh. We also have to check if @pendingReply is not NULL before proceeding any further. Signed-off-by: Michal Privoznik --- src/rpc/virnetdaemon.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index e805e3a3c..00247cfc3 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -471,6 +471,7 @@ virNetDaemonGotInhibitReply(DBusPendingCall *pending, =20 cleanup: virObjectUnlock(dmn); + dbus_pending_call_unref(pending); } =20 =20 @@ -483,7 +484,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn, const char *mode) { DBusMessage *message; - DBusPendingCall *pendingReply; + DBusPendingCall *pendingReply =3D NULL; DBusConnection *systemBus; =20 VIR_DEBUG("dmn=3D%p what=3D%s who=3D%s why=3D%s mode=3D%s", @@ -510,13 +511,17 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn, DBUS_TYPE_STRING, &mode, DBUS_TYPE_INVALID); =20 - pendingReply =3D NULL; if (dbus_connection_send_with_reply(systemBus, message, &pendingReply, - 25*1000)) { - dbus_pending_call_set_notify(pendingReply, - virNetDaemonGotInhibitReply, - dmn, NULL); + 25 * 1000) && + pendingReply) { + if (dbus_pending_call_get_completed(pendingReply)) { + virNetDaemonGotInhibitReply(pendingReply, dmn); + } else { + dbus_pending_call_set_notify(pendingReply, + virNetDaemonGotInhibitReply, + dmn, NULL); + } dmn->autoShutdownCallingInhibit =3D true; } virDBusMessageUnref(message); --=20 2.13.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list