[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Stored secrets seem to get corrupted





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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]