From nobody Fri Apr 19 09:24:31 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) client-ip=205.139.110.61; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1596041853; cv=none; d=zohomail.com; s=zohoarc; b=nLt5TROvF2nci3Vm3FoWkyrNxuaXp1vGOlTGMfvrOnKTxEEN6sxYyFevJhG/Ex4hMny0lxMfEG2CSBxdzNz8KTfoLcXPSj5iIboIAgBxoy+/rEjrBvOGxjGoqMsXG1ak25PklrfklV1ZwdvX4LHARd6X1mwG0C/Rhc4LnVaQHbw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1596041853; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=zdpUei5YPqNEd++kyjo584iDqPD8MA83N+jw3aVzMSU=; b=fpF7kJQUti42SENluhzwedr02QoJLZTw+dTz2B3e8vhJ29IKTuGQrNoQENBVF6utqG/zqPq6TL3arnFQCBorOCnXdPV6NgV2gKFGRX6UVfPrRjAWm4PJF8zzMdcTfTedqctZ4oDkOyZagDhDgNsCRfywvlmaUcVIfSEldGNvB14= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by mx.zohomail.com with SMTPS id 1596041853340920.1256485058594; Wed, 29 Jul 2020 09:57:33 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-398-QwupDZN1NveQJCP3IKEMIg-1; Wed, 29 Jul 2020 12:57:04 -0400 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id C9226106B246; Wed, 29 Jul 2020 16:56:58 +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 7C9338A17A; Wed, 29 Jul 2020 16:56:57 +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 3C92095A62; Wed, 29 Jul 2020 16:56:53 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 06TGupB6013676 for ; Wed, 29 Jul 2020 12:56:51 -0400 Received: by smtp.corp.redhat.com (Postfix) id D17831992D; Wed, 29 Jul 2020 16:56:51 +0000 (UTC) Received: from domokun.gsslab.fab.redhat.com (unknown [10.33.8.110]) by smtp.corp.redhat.com (Postfix) with ESMTP id B273919D9E; Wed, 29 Jul 2020 16:56:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1596041851; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=zdpUei5YPqNEd++kyjo584iDqPD8MA83N+jw3aVzMSU=; b=BkcnXBTrOtTPEa3QQQCTJOdfwdVsrx8D57fB9bGc37ZundywTbedtD/jTV9OrE9f1NmZZE q9vC7YP+xXaC0/O3FkvdRRyerh/qgQD7xUoLbBb8qP2gIFfc+d+nXyzxwaYTOVOyKIphiL jxV5o1gCAjk6UgrvFJXTXvFWaSfdXpE= X-MC-Unique: QwupDZN1NveQJCP3IKEMIg-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [PATCH] util: avoid race in releasing the GSource in event thread Date: Wed, 29 Jul 2020 17:56:43 +0100 Message-Id: <20200729165643.2742394-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com 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: , 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-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) There is a race between vir_event_thread_finalize and virEventThreadWorker in releasing the last reference on the GMainContext. If virEventThreadDataFree() runs after vir_event_thread_finalize releases its reference, then it will release the last reference on the GMainContext. As a result g_autoptr cleanup on the GSource will access free'd memory. The race can be seen in non-deterministic crashes of the virt-run-qemu program during its shutdown, but could also likely affect the main libvirtd QEMU driver: Thread 2 (Thread 0x7f508ffff700 (LWP 222813)): #0 0x00007f509c8e26b0 in malloc_consolidate (av=3Dav@entry=3D0x7f5088000= 020) at malloc.c:4488 #1 0x00007f509c8e4b08 in _int_malloc (av=3Dav@entry=3D0x7f5088000020, by= tes=3Dbytes@entry=3D2048) at malloc.c:3711 #2 0x00007f509c8e6412 in __GI___libc_malloc (bytes=3D2048) at malloc.c:3= 073 #3 0x00007f509d6e925e in g_realloc (mem=3D0x0, n_bytes=3D2048) at gmem.c= :164 #4 0x00007f509d705a57 in g_string_maybe_expand (string=3Dstring@entry=3D= 0x7f5088001f20, len=3Dlen@entry=3D1024) at gstring.c:102 #5 0x00007f509d705ab6 in g_string_sized_new (dfl_size=3Ddfl_size@entry= =3D1024) at gstring.c:127 #6 0x00007f509d708c5e in g_test_log_dump (len=3D, msg= =3D) at gtestutils.c:3330 #7 0x00007f509d708c5e in g_test_log (lbit=3DG_TEST_LOG_ERROR, string1=3D0x7f508800fcb0 "GLib:ERROR:ghash.= c:377:g_hash_table_lookup_node: assertion failed: (hash_table->ref_count > = 0)", string2=3D, n_args=3D0, largs=3D0x0) at gtestutils.c:975 #8 0x00007f509d70af2a in g_assertion_message (domain=3D, file=3D0x7f509d7324a2 "ghash.c", line=3D, func=3D0x7f509d732750 <__func__.11348> "g_hash_table_lookup_= node", message=3D) at gtestutils.c:2504 #9 0x00007f509d70af8e in g_assertion_message_expr (domain=3Ddomain@entry=3D0x7f509d72d76e "GLib", file=3Dfile@entry=3D0= x7f509d7324a2 "ghash.c", line=3Dline@entry=3D377, func=3Dfunc@entry=3D0x7f5= 09d732750 <__func__.11348> "g_hash_table_lookup_node", expr=3Dexpr@entry=3D= 0x7f509d732488 "hash_table->ref_count > 0") at gtestutils.c:2555 #10 0x00007f509d6d197e in g_hash_table_lookup_node (hash_table=3D0x55b70a= ce1760, key=3D, hash_return=3D) at ghash.= c:377 #11 0x00007f509d6d197e in g_hash_table_lookup_node (hash_return=3D, key=3D, hash_table=3D0x55b70ace1760) at ghash.= c:361 #12 0x00007f509d6d197e in g_hash_table_remove_internal (hash_table=3D0x55= b70ace1760, key=3D, notify=3D1) at ghash.c:1371 #13 0x00007f509d6e0664 in g_source_unref_internal (source=3D0x7f5088000b6= 0, context=3D0x55b70ad87e00, have_lock=3D0) at gmain.c:2103 #14 0x00007f509d6e1f64 in g_source_unref (source=3D) at gm= ain.c:2176 #15 0x00007f50a08ff84c in glib_autoptr_cleanup_GSource (_ptr=3D) at /usr/include/glib-2.0/glib/glib-autocleanups.h:58 #16 0x00007f50a08ff84c in virEventThreadWorker (opaque=3D0x55b70ad87f80) = at ../../src/util/vireventthread.c:114 #17 0x00007f509d70bd4a in g_thread_proxy (data=3D0x55b70acf3850) at gthre= ad.c:784 #18 0x00007f509d04714a in start_thread (arg=3D) at pthread= _create.c:479 #19 0x00007f509c95cf23 in clone () at ../sysdeps/unix/sysv/linux/x86_64/c= lone.S:95 Thread 1 (Thread 0x7f50a1380c00 (LWP 222802)): #0 0x00007f509c8977ff in __GI_raise (sig=3Dsig@entry=3D6) at ../sysdeps/= unix/sysv/linux/raise.c:50 #1 0x00007f509c881c35 in __GI_abort () at abort.c:79 #2 0x00007f509d72a823 in g_mutex_clear (mutex=3D0x55b70ad87e00) at gthre= ad-posix.c:1307 #3 0x00007f509d72a823 in g_mutex_clear (mutex=3Dmutex@entry=3D0x55b70ad8= 7e00) at gthread-posix.c:1302 #4 0x00007f509d6e1a84 in g_main_context_unref (context=3D0x55b70ad87e00)= at gmain.c:582 #5 0x00007f509d6e1a84 in g_main_context_unref (context=3D0x55b70ad87e00)= at gmain.c:541 #6 0x00007f50a08ffabb in vir_event_thread_finalize (object=3D0x55b70ad83= 180 [virEventThread]) at ../../src/util/vireventthread.c:50 #7 0x00007f509d9c48a9 in g_object_unref (_object=3D) at g= object.c:3340 #8 0x00007f509d9c48a9 in g_object_unref (_object=3D0x55b70ad83180) at go= bject.c:3232 #9 0x00007f509583d311 in qemuProcessQMPFree (proc=3Dproc@entry=3D0x55b70= ad87b90) at ../../src/qemu/qemu_process.c:8355 #10 0x00007f5095790f58 in virQEMUCapsInitQMPSingle (qemuCaps=3DqemuCaps@entry=3D0x55b70ad88010, libDir=3DlibDir@entry=3D= 0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=3DrunUid@entry= =3D107, runGid=3DrunGid@entry=3D107, onlyTCG=3DonlyTCG@entry=3Dfalse) at ..= /../src/qemu/qemu_capabilities.c:5409 #11 0x00007f509579108f in virQEMUCapsInitQMP (runGid=3D107, runUid=3D107,= libDir=3D0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", qemuCaps=3D0= x55b70ad88010) at ../../src/qemu/qemu_capabilities.c:5420 #12 0x00007f509579108f in virQEMUCapsNewForBinaryInternal (hostArch=3DVIR_ARCH_X86_64, binary=3Dbinary@entry=3D0x55b70ad7dc40 "= /usr/libexec/qemu-kvm", libDir=3D0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/= lib/qemu", runUid=3D107, runGid=3D107, hostCPUSignature=3D0x55b70ad01320 "G= enuineIntel, Intel(R) Xeon(R) Silver 4210 CPU @ 2.20GHz, family: 6, model: = 85, stepping: 7", microcodeVersion=3D83898113, kernelVersion=3D0x55b70ad00d= 60 "4.18.0-211.el8.x86_64 #1 SMP Thu Jun 4 08:08:16 UTC 2020") at ../../src= /qemu/qemu_capabilities.c:5472 #13 0x00007f5095791373 in virQEMUCapsNewData (binary=3D0x55b70ad7dc40 "/u= sr/libexec/qemu-kvm", privData=3D0x55b70ad5b8f0) at ../../src/qemu/qemu_cap= abilities.c:5505 #14 0x00007f50a09a32b1 in virFileCacheNewData (name=3D0x55b70ad7dc40 "/us= r/libexec/qemu-kvm", cache=3D) at ../../src/util/virfilecach= e.c:208 #15 0x00007f50a09a32b1 in virFileCacheValidate (cache=3Dcache@entry=3D0x5= 5b70ad5c030, name=3Dname@entry=3D0x55b70ad7dc40 "/usr/libexec/qemu-kvm", da= ta=3Ddata@entry=3D0x7ffca39ffd90) at ../../src/util/virfilecache.c:277 #16 0x00007f50a09a37ea in virFileCacheLookup (cache=3Dcache@entry=3D0x55b= 70ad5c030, name=3Dname@entry=3D0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at .= ./../src/util/virfilecache.c:310 #17 0x00007f5095791627 in virQEMUCapsCacheLookup (cache=3D0x55b70ad5c030,= binary=3D0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/qemu/qemu_ca= pabilities.c:5647 #18 0x00007f50957c34c3 in qemuDomainPostParseDataAlloc (def=3D, parseFlags=3D, opaque=3D, parseOpaque= =3D0x7ffca39ffe18) at ../../src/qemu/qemu_domain.c:5470 #19 0x00007f50a0a34051 in virDomainDefPostParse (def=3Ddef@entry=3D0x55b70ad7d200, parseFlags=3DparseFlags@entry=3D25= 8, xmlopt=3Dxmlopt@entry=3D0x55b70ad5d010, parseOpaque=3DparseOpaque@entry= =3D0x0) at ../../src/conf/domain_conf.c:5970 #20 0x00007f50a0a464bb in virDomainDefParseNode (xml=3Dxml@entry=3D0x55b70aced140, root=3Droot@entry=3D0x55b70ad5f020= , xmlopt=3Dxmlopt@entry=3D0x55b70ad5d010, parseOpaque=3DparseOpaque@entry= =3D0x0, flags=3Dflags@entry=3D258) at ../../src/conf/domain_conf.c:22520 #21 0x00007f50a0a4669b in virDomainDefParse (xmlStr=3DxmlStr@entry=3D0x55b70ad5f9e0 "\n 83\n 9350639d-1c8a-4f51-a4a6-4eaf8eabe83e\n \n \n <"..., filename=3Dfilename@entry=3D0x0, xml= opt=3D0x55b70ad5d010, parseOpaque=3DparseOpaque@entry=3D0x0, flags=3Dflags@= entry=3D258) at ../../src/conf/domain_conf.c:22474 #22 0x00007f50a0a467ae in virDomainDefParseString (xmlStr=3DxmlStr@entry=3D0x55b70ad5f9e0 "\n 83\n 9350639d-1c8a-4f51-a4a6-4eaf8eabe83e\n \n \n <"..., xmlopt=3D, parseOpaqu= e=3DparseOpaque@entry=3D0x0, flags=3Dflags@entry=3D258) at ../../src/conf/domain_conf.c:22488 #23 0x00007f50958ce112 in qemuDomainCreateXML (conn=3D0x55b70acf9090, xml=3D0x55b70ad5f9e0 "\n= 83\n 9350639d-1c8a-4f51-a4a6-4eaf8eabe83e\n <= metadata>\n \n <"..., flags=3D0) at ../../src/qemu/qe= mu_driver.c:1744 #24 0x00007f50a0c268ac in virDomainCreateXML (conn=3D0x55b70acf9090, xmlDesc=3D0x55b70ad5f9e0 "\n 83\n 9350639d-1c8a-4f51-a4a6-4eaf8eabe83e\= n \n \n <"..., flags=3D0) at ../../src/lib= virt-domain.c:176 #25 0x000055b709547e7b in main (argc=3D, argv=3D) at ../../src/qemu/qemu_shim.c:289 The solution is to explicitly unref the GSource at a safe time instead of letting g_autoptr unref it when leaving scope. Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: Michal Privoznik --- src/util/vireventthread.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util/vireventthread.c b/src/util/vireventthread.c index cf865925eb..485672278a 100644 --- a/src/util/vireventthread.c +++ b/src/util/vireventthread.c @@ -111,7 +111,11 @@ static void * virEventThreadWorker(void *opaque) { virEventThreadData *data =3D opaque; - g_autoptr(GSource) running =3D g_idle_source_new(); + /* + * Do NOT use g_autoptr on this. We need to unref it + * before the GMainContext is unrefed + */ + GSource *running =3D g_idle_source_new(); =20 g_source_set_callback(running, virEventThreadNotify, data, NULL); =20 @@ -119,6 +123,7 @@ virEventThreadWorker(void *opaque) =20 g_main_loop_run(data->loop); =20 + g_source_unref(running); virEventThreadDataFree(data); =20 return NULL; --=20 2.24.1