Re: [Libguestfs] [PATCH nbdkit] wrapper: Set MALLOC_CHECK=1 and MALLOC_PERTURB_ (randomly).
On Mon, Mar 18, 2019 at 12:29:02PM -0500, Eric Blake wrote:
On 3/18/19 11:59 AM, Richard W.M. Jones wrote:
> This is a cheap way to find some use-after-free and uninitialized read
> problems when using glibc.
>
> This in fact reveals a race during filter shutdown (which this
> commit does not fix):
>
> Thread 2 (Thread 0x7f1caaa5ffc0 (LWP 7223)):
> #0 0x00007f1cab0a05f8 in pthread_rwlock_wrlock () from /lib64/libpthread.so.0
> #1 0x0000000000408842 in lock_unload () at locks.c:97
> #2 0x00000000004066ff in filter_free (b=0x203c330) at filters.c:77
Do we need some sort of pthread_join() in filter_free() to allow...
> #3 0x000000000040a6f4 in main (argc=11, argv=0x7ffc1f4486e8) at main.c:649
>
> Thread 1 (Thread 0x7f1caaa5e700 (LWP 7226)):
> #0 0x000000000040732a in filter_finalize (b=0x203c330, conn=0x203d870)
> at filters.c:421
...filter_finalize() time to finish its job? But I agree that solving
the race is an independent patch.
I don't think there's a very simple answer here. On the other hand
I've long known that there are problems with nbdkit along the signal /
shutdown path, and it needs attention at some point.
Rich.
> #1 0x0000000000404d07 in _handle_single_connection (sockin=6,
sockout=6)
> at connections.c:239
> #2 0x0000000000404d76 in handle_single_connection (sockin=6, sockout=6)
> at connections.c:258
> #3 0x00000000004119f6 in start_thread (datav=0x203b450) at sockets.c:263
> #4 0x00007f1cab09b5a2 in start_thread () from /lib64/libpthread.so.0
> #5 0x00007f1caafc85c3 in clone () from /lib64/libc.so.6
>
> What's happening here is that the filter / plugin chain is freed by
> Thread 2, while Thread 1 is calling filter->finalize. At this point
> filter->finalize points to freed memory, but "normally" this would
> still contain the correct function pointer. However when
> MALLOC_PERTURB_ is set the function pointer is overwritten with the
> non-zero dead memory pattern which we then attempt to call, causing a
> segfault (in Thread 1).
> ---
> wrapper.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
I like that it makes in-tree testing more robust, without penalizing
release builds.
ACK.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v