[libvirt] Stored secrets seem to get corrupted

Wed Jul 4 09:13:54 UTC 2012

On 04-07-12 10:45, Daniel P. Berrange wrote:
> On Wed, Jul 04, 2012 at 10:24:38AM +0200, Wido den Hollander wrote:
>>
>>
>> On 03-07-12 17:21, Daniel P. Berrange wrote:
>>> On Tue, Jul 03, 2012 at 05:11:48PM +0200, Wido den Hollander wrote:
>>>> Yes, there is memory corruption somewhere. I never used valgrind
>>>> before, but the output seems to show.
>>>>
>>>> I ran libvirtd inside a screen, I've attached the screenlog with all
>>>> the output.
>>>>
>>>> At the end you'll see there is a misread. I was storing the base64
>>>> encoded value of "wido".
>>>
>>> Thanks so there are two errors shown by valgrind
>>>
>>> ==6825== Invalid read of size 1
>>> ==6825==    at 0x5769DBA: vfprintf (vfprintf.c:1624)
>>> ==6825==    by 0x58289D0: __vasprintf_chk (vasprintf_chk.c:68)
>>> ==6825==    by 0x509C727: virVasprintf (stdio2.h:199)
>>> ==6825==    by 0x508CFEA: virLogVMessage (logging.c:749)
>>> ==6825==    by 0x508D349: virLogMessage (logging.c:696)
>>> ==6825==    by 0x127AB0BC: secretSaveValue (secret_driver.c:297)
>>> ==6825==    by 0x127AB30A: secretSetValue (secret_driver.c:861)
>>> ==6825==    by 0x5130CF4: virSecretSetValue (libvirt.c:14457)
>>> ==6825==    by 0x41CC8F: remoteDispatchSecretSetValueHelper (remote_dispatch.h:10962)
>>> ==6825==    by 0x5174F84: virNetServerProgramDispatch (virnetserverprogram.c:416)
>>> ==6825==    by 0x5171260: virNetServerHandleJob (virnetserver.c:161)
>>> ==6825==    by 0x50990AD: virThreadPoolWorker (threadpool.c:143)
>>> ==6825==  Address 0x19cb3c64 is 0 bytes after a block of size 4 alloc'd
>>> ==6825==    at 0x4C29DB4: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==6825==    by 0x508E32F: virAllocN (memory.c:128)
>>> ==6825==    by 0x127AB21B: secretSetValue (secret_driver.c:838)
>>> ==6825==    by 0x5130CF4: virSecretSetValue (libvirt.c:14457)
>>> ==6825==    by 0x41CC8F: remoteDispatchSecretSetValueHelper (remote_dispatch.h:10962)
>>> ==6825==    by 0x5174F84: virNetServerProgramDispatch (virnetserverprogram.c:416)
>>> ==6825==    by 0x5171260: virNetServerHandleJob (virnetserver.c:161)
>>> ==6825==    by 0x50990AD: virThreadPoolWorker (threadpool.c:143)
>>> ==6825==    by 0x5098755: virThreadHelper (threads-pthread.c:161)
>>> ==6825==    by 0x550AE99: start_thread (pthread_create.c:308)
>>> ==6825==    by 0x58124BC: clone (clone.S:112)
>>>
>>> This one is harmless - this is because the VIR_DEBUG line you added
>>> uses '%s' to print secret_value, but this is not a NULL terminated
>>> string. So we can ignore this.
>>>
>>>
>>> ==6825==
>>> ==6825== Invalid read of size 4
>>> ==6825==    at 0xA57E4B9: base64_encode (in /usr/lib/x86_64-linux-gnu/libroken.so.18.1.0)
>>> ==6825==    by 0x10DDBC98: base64_encode_alloc (base64.c:140)
>>> ==6825==    by 0x127AB0EF: secretSaveValue (secret_driver.c:302)
>>> ==6825==    by 0x127AB30A: secretSetValue (secret_driver.c:861)
>>> ==6825==    by 0x5130CF4: virSecretSetValue (libvirt.c:14457)
>>> ==6825==    by 0x41CC8F: remoteDispatchSecretSetValueHelper (remote_dispatch.h:10962)
>>> ==6825==    by 0x5174F84: virNetServerProgramDispatch (virnetserverprogram.c:416)
>>> ==6825==    by 0x5171260: virNetServerHandleJob (virnetserver.c:161)
>>> ==6825==    by 0x50990AD: virThreadPoolWorker (threadpool.c:143)
>>> ==6825==    by 0x5098755: virThreadHelper (threads-pthread.c:161)
>>> ==6825==    by 0x550AE99: start_thread (pthread_create.c:308)
>>> ==6825==    by 0x58124BC: clone (clone.S:112)
>>> ==6825==  Address 0xb7786c8 is 8 bytes inside a block of size 9 alloc'd
>>> ==6825==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==6825==    by 0xA57E3D7: base64_encode (in /usr/lib/x86_64-linux-gnu/libroken.so.18.1.0)
>>> ==6825==    by 0x10DDBC98: base64_encode_alloc (base64.c:140)
>>> ==6825==    by 0x127AB0EF: secretSaveValue (secret_driver.c:302)
>>> ==6825==    by 0x127AB30A: secretSetValue (secret_driver.c:861)
>>> ==6825==    by 0x5130CF4: virSecretSetValue (libvirt.c:14457)
>>> ==6825==    by 0x41CC8F: remoteDispatchSecretSetValueHelper (remote_dispatch.h:10962)
>>> ==6825==    by 0x5174F84: virNetServerProgramDispatch (virnetserverprogram.c:416)
>>> ==6825==    by 0x5171260: virNetServerHandleJob (virnetserver.c:161)
>>> ==6825==    by 0x50990AD: virThreadPoolWorker (threadpool.c:143)
>>>
>>> This one is very interesting. It shows that the 'base64_encode' function
>>> is doing an out-of-bounds read. More tellingly though is that it is
>>> reporting 'base64_encode' function is in a wierd library:
>>>
>>>     /usr/lib/x86_64-linux-gnu/libroken.so.18.1.
>>>
>>> If this were normal, we should expect to see that function present
>>> in 'base64.c' since this function code is provided by gnulib itself.
>>>
>>> So something else libvirt is linking to, directly or indirectly
>>> is using  libroken.so which also has a 'base64_encode'symbol
>>> defined. This is overriding gnulib's symbol of the same name.
>>>
>>> I'm willing to bet the API contract of this libroken.so base64_encode.
>>> differs from GNULIBS, with crashtastic results
>>>
>>> Can you try and find how this libroken.so is getting linked to libvirt ?
>>>
>>> Or indeed what this library does, or what package it is part of.
>>
>> The library is libroken18-heimdal under Ubuntu 12.04:
>> http://packages.ubuntu.com/precise/libroken18-heimdal
>>
>> When installing ubuntu-virt-server libraries like gnutls depend on
>> this library.
>>
>> I'm not sure why libvirt gets linked to libroken, but on Ubuntu
>> systems it's installed on most system which use libvirt.
>>
>> I haven't found a header file which as the symbol 'base64_encode' in
>> it for libroken, only gnutls and glib seem to have something that
>> has 'base64_encode' in it, but those are prefixed with g_
>
> I expect that this is an internal symbol from libroken.so which
> they leak into the public namespace.

I just verified by downloading the source from http://www.h5l.org and 
lib/roken/base64.h indeed declares base64_encode and base64_decode.

>
>> For now I have no clues to why it gets linked to libroken, but it
>> seems the problem has been found.
>
> If gnutls links to libroken.so, then libvirt gets linked to it
> indirectly
>
> It sounds like we might need to have a workaround in gnulib to
> avoid this problem. With other cases where gnulib replaces existing
> symbols they use some magic such that the gnulib replacement gets
> prefixed with 'rpl_'.

Not my expertise, but sounds reasonable. It will take time however I think?

Until then secrets on Ubuntu 12.04 will be broken.

Wido