[PATCH] util: avoid race in releasing the GSource in event thread

Michal Privoznik mprivozn at redhat.com
Thu Jul 30 08:14:07 UTC 2020


On 7/29/20 6:56 PM, Daniel P. Berrangé wrote:
> 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=av at entry=0x7f5088000020) at malloc.c:4488
>    #1  0x00007f509c8e4b08 in _int_malloc (av=av at entry=0x7f5088000020, bytes=bytes at entry=2048) at malloc.c:3711
>    #2  0x00007f509c8e6412 in __GI___libc_malloc (bytes=2048) at malloc.c:3073
>    #3  0x00007f509d6e925e in g_realloc (mem=0x0, n_bytes=2048) at gmem.c:164
>    #4  0x00007f509d705a57 in g_string_maybe_expand (string=string at entry=0x7f5088001f20, len=len at entry=1024) at gstring.c:102
>    #5  0x00007f509d705ab6 in g_string_sized_new (dfl_size=dfl_size at entry=1024) at gstring.c:127
>    #6  0x00007f509d708c5e in g_test_log_dump (len=<synthetic pointer>, msg=<synthetic pointer>) at gtestutils.c:3330
>    #7  0x00007f509d708c5e in g_test_log
>        (lbit=G_TEST_LOG_ERROR, string1=0x7f508800fcb0 "GLib:ERROR:ghash.c:377:g_hash_table_lookup_node: assertion failed: (hash_table->ref_count > 0)", string2=<optimized out>, n_args=0, largs=0x0) at gtestutils.c:975
>    #8  0x00007f509d70af2a in g_assertion_message
>        (domain=<optimized out>, file=0x7f509d7324a2 "ghash.c", line=<optimized out>, func=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", message=<optimized out>)
>        at gtestutils.c:2504
>    #9  0x00007f509d70af8e in g_assertion_message_expr
>        (domain=domain at entry=0x7f509d72d76e "GLib", file=file at entry=0x7f509d7324a2 "ghash.c", line=line at entry=377, func=func at entry=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", expr=expr at entry=0x7f509d732488 "hash_table->ref_count > 0") at gtestutils.c:2555
>    #10 0x00007f509d6d197e in g_hash_table_lookup_node (hash_table=0x55b70ace1760, key=<optimized out>, hash_return=<synthetic pointer>) at ghash.c:377
>    #11 0x00007f509d6d197e in g_hash_table_lookup_node (hash_return=<synthetic pointer>, key=<optimized out>, hash_table=0x55b70ace1760) at ghash.c:361
>    #12 0x00007f509d6d197e in g_hash_table_remove_internal (hash_table=0x55b70ace1760, key=<optimized out>, notify=1) at ghash.c:1371
>    #13 0x00007f509d6e0664 in g_source_unref_internal (source=0x7f5088000b60, context=0x55b70ad87e00, have_lock=0) at gmain.c:2103
>    #14 0x00007f509d6e1f64 in g_source_unref (source=<optimized out>) at gmain.c:2176
>    #15 0x00007f50a08ff84c in glib_autoptr_cleanup_GSource (_ptr=<synthetic pointer>) at /usr/include/glib-2.0/glib/glib-autocleanups.h:58
>    #16 0x00007f50a08ff84c in virEventThreadWorker (opaque=0x55b70ad87f80) at ../../src/util/vireventthread.c:114
>    #17 0x00007f509d70bd4a in g_thread_proxy (data=0x55b70acf3850) at gthread.c:784
>    #18 0x00007f509d04714a in start_thread (arg=<optimized out>) at pthread_create.c:479
>    #19 0x00007f509c95cf23 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
> 
>    Thread 1 (Thread 0x7f50a1380c00 (LWP 222802)):
>    #0  0x00007f509c8977ff in __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
>    #1  0x00007f509c881c35 in __GI_abort () at abort.c:79
>    #2  0x00007f509d72a823 in g_mutex_clear (mutex=0x55b70ad87e00) at gthread-posix.c:1307
>    #3  0x00007f509d72a823 in g_mutex_clear (mutex=mutex at entry=0x55b70ad87e00) at gthread-posix.c:1302
>    #4  0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:582
>    #5  0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:541
>    #6  0x00007f50a08ffabb in vir_event_thread_finalize (object=0x55b70ad83180 [virEventThread]) at ../../src/util/vireventthread.c:50
>    #7  0x00007f509d9c48a9 in g_object_unref (_object=<optimized out>) at gobject.c:3340
>    #8  0x00007f509d9c48a9 in g_object_unref (_object=0x55b70ad83180) at gobject.c:3232
> 
>    #9  0x00007f509583d311 in qemuProcessQMPFree (proc=proc at entry=0x55b70ad87b90) at ../../src/qemu/qemu_process.c:8355
>    #10 0x00007f5095790f58 in virQEMUCapsInitQMPSingle
>        (qemuCaps=qemuCaps at entry=0x55b70ad88010, libDir=libDir at entry=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=runUid at entry=107, runGid=runGid at entry=107, onlyTCG=onlyTCG at entry=false) at ../../src/qemu/qemu_capabilities.c:5409
>    #11 0x00007f509579108f in virQEMUCapsInitQMP (runGid=107, runUid=107, libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", qemuCaps=0x55b70ad88010)
>        at ../../src/qemu/qemu_capabilities.c:5420
>    #12 0x00007f509579108f in virQEMUCapsNewForBinaryInternal
>        (hostArch=VIR_ARCH_X86_64, binary=binary at entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=107, runGid=107, hostCPUSignature=0x55b70ad01320 "GenuineIntel, Intel(R) Xeon(R) Silver 4210 CPU @ 2.20GHz, family: 6, model: 85, stepping: 7", microcodeVersion=83898113, kernelVersion=0x55b70ad00d60 "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=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", privData=0x55b70ad5b8f0) at ../../src/qemu/qemu_capabilities.c:5505
>    #14 0x00007f50a09a32b1 in virFileCacheNewData (name=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", cache=<optimized out>) at ../../src/util/virfilecache.c:208
>    #15 0x00007f50a09a32b1 in virFileCacheValidate (cache=cache at entry=0x55b70ad5c030, name=name at entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", data=data at entry=0x7ffca39ffd90)
>        at ../../src/util/virfilecache.c:277
>    #16 0x00007f50a09a37ea in virFileCacheLookup (cache=cache at entry=0x55b70ad5c030, name=name at entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/util/virfilecache.c:310
>    #17 0x00007f5095791627 in virQEMUCapsCacheLookup (cache=0x55b70ad5c030, binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/qemu/qemu_capabilities.c:5647
>    #18 0x00007f50957c34c3 in qemuDomainPostParseDataAlloc (def=<optimized out>, parseFlags=<optimized out>, opaque=<optimized out>, parseOpaque=0x7ffca39ffe18)
>        at ../../src/qemu/qemu_domain.c:5470
>    #19 0x00007f50a0a34051 in virDomainDefPostParse
>        (def=def at entry=0x55b70ad7d200, parseFlags=parseFlags at entry=258, xmlopt=xmlopt at entry=0x55b70ad5d010, parseOpaque=parseOpaque at entry=0x0)
>        at ../../src/conf/domain_conf.c:5970
>    #20 0x00007f50a0a464bb in virDomainDefParseNode
>        (xml=xml at entry=0x55b70aced140, root=root at entry=0x55b70ad5f020, xmlopt=xmlopt at entry=0x55b70ad5d010, parseOpaque=parseOpaque at entry=0x0, flags=flags at entry=258)
>        at ../../src/conf/domain_conf.c:22520
>    #21 0x00007f50a0a4669b in virDomainDefParse
>        (xmlStr=xmlStr at entry=0x55b70ad5f9e0 "<domain type='kvm'>\n  <name>83</name>\n  <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n      <"..., filename=filename at entry=0x0, xmlopt=0x55b70ad5d010, parseOpaque=parseOpaque at entry=0x0, flags=flags at entry=258) at ../../src/conf/domain_conf.c:22474
>    #22 0x00007f50a0a467ae in virDomainDefParseString
>        (xmlStr=xmlStr at entry=0x55b70ad5f9e0 "<domain type='kvm'>\n  <name>83</name>\n  <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n      <"..., xmlopt=<optimized out>, parseOpaque=parseOpaque at entry=0x0, flags=flags at entry=258)
>        at ../../src/conf/domain_conf.c:22488
>    #23 0x00007f50958ce112 in qemuDomainCreateXML
>        (conn=0x55b70acf9090, xml=0x55b70ad5f9e0 "<domain type='kvm'>\n  <name>83</name>\n  <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n      <"..., flags=0) at ../../src/qemu/qemu_driver.c:1744
>    #24 0x00007f50a0c268ac in virDomainCreateXML
>        (conn=0x55b70acf9090, xmlDesc=0x55b70ad5f9e0 "<domain type='kvm'>\n  <name>83</name>\n  <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n      <"..., flags=0) at ../../src/libvirt-domain.c:176
>    #25 0x000055b709547e7b in main (argc=<optimized out>, argv=<optimized out>) 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é <berrange at redhat.com>
> ---
>   src/util/vireventthread.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal




More information about the libvir-list mailing list