On Tue, 2019-07-09 at 10:21 +0100, Daniel P. Berrangé wrote:
> On Mon, Jul 08, 2019 at 06:03:10PM +0200, Andrea Bolognani wrote:
> > When only geteuid() is mocked, the test crashes on Debian 10.
> >
> > Fatal: failed to reset uid: No such file or directory
> >
> > Program received signal SIGABRT, Aborted.
> > __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> > 50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
> > (gdb) t a a bt
> >
> > Thread 1 (Thread 0x7ffff3b3e080 (LWP 12003)):
> > #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> > #1 0x00007ffff7798535 in __GI_abort () at abort.c:79
> > #2 0x00007ffff485ca20 in _gcry_logv (level=level@entry=40, fmt=fmt@entry=0x7ffff4929126 "failed to reset uid: %s\n", arg_ptr=arg_ptr@entry=0x7fffffffe4a0) at ../../src/misc.c:142
> > #3 0x00007ffff485cd61 in _gcry_log_fatal (fmt=fmt@entry=0x7ffff4929126 "failed to reset uid: %s\n") at ../../src/misc.c:218
> > #4 0x00007ffff48639d1 in lock_pool_pages (n=<optimized out>, p=<optimized out>) at ../../src/secmem.c:340
> > #5 _gcry_secmem_init_internal (n=<optimized out>) at ../../src/secmem.c:563
> > #6 0x00007ffff4863d78 in _gcry_secmem_init (n=4096) at ../../src/secmem.c:581
> > #7 0x00007ffff485e4e6 in _gcry_vcontrol (cmd=<optimized out>, arg_ptr=arg_ptr@entry=0x7fffffffe5e0) at ../../src/global.c:506
> > #8 0x00007ffff485a789 in gcry_control (cmd=cmd@entry=GCRYCTL_INIT_SECMEM) at ../../src/visibility.c:79
> > #9 0x00007ffff71af10f in ssh_crypto_init () at ./src/libgcrypt.c:621
> > #10 0x00007ffff7193796 in _ssh_init (constructor=constructor@entry=1) at ./src/init.c:79
> > #11 0x00007ffff71834de in libssh_constructor () at ./src/init.c:116
>
> Ewww, so its crashing in a ELF library constructor for libssh, which
> is in turn calling into libgcrypt.
>
> Obviously nothing todo with the test cases we're actually running.
> I guess we're confusing the code into thinking it has some wierd
> privilege by having geteuid() return 0, while getuid() returns
> the normal UID.
>
> Mocking getuid() is ok, but I fear its just a targetting one
> specific problem we happen to be hitting today.
> So I think we probably ought to make geteuid() delegate the
> real getuid() function instead. Have a global flag we can set
> & unset just before & after executing the real test code. That
> way libraries will see correct UID info in general.
I tried making the mocking more selective (see attached patch) but
that doesn't work because there are two times we need the mocking to
be in place: when calling virNetDevBandwidthSet(), which errors out
if it detects it's not running as root, and when the libssh is
initialized, which happens automatically when the library is loaded.
The attached patch only covers the former, and extending it so that
it also does the latter seems like it would be a complete mess; plus
at that point there wouldn't be a lot of the test program still
running without mocking, so I wonder what we'd gain by going through
all that trouble.
Unless, of course, I've completely misunderstood what you were
suggesting in the first place O:-)
--
Andrea Bolognani / Red Hat / Virtualization
From 75519a4efbde4e19ee7820cd74bbeb607b5c1b97 Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <abologna@xxxxxxxxxx>
Date: Tue, 9 Jul 2019 12:49:25 +0200
Subject: [PATCH] tests: Mock get(e)uid() more selectively
Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
---
tests/virnetdevbandwidthmock.c | 11 +++++++++--
tests/virnetdevbandwidthtest.c | 4 ++++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/tests/virnetdevbandwidthmock.c b/tests/virnetdevbandwidthmock.c
index f0c6b22c5f..70c58e0cdf 100644
--- a/tests/virnetdevbandwidthmock.c
+++ b/tests/virnetdevbandwidthmock.c
@@ -19,13 +19,20 @@
#include <config.h>
#include <unistd.h>
#include <sys/types.h>
+#include <stdlib.h>
uid_t geteuid(void)
{
- return 0;
+ if (getenv("VIR_TEST_MOCK_GETUID"))
+ return 0;
+ else
+ return geteuid();
}
uid_t getuid(void)
{
- return 0;
+ if (getenv("VIR_TEST_MOCK_GETUID"))
+ return 0;
+ else
+ return getuid();
}
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 2c0b6a6713..dacc611653 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -80,9 +80,13 @@ testVirNetDevBandwidthSet(const void *data)
virCommandSetDryRun(&buf, NULL, NULL);
+ setenv("VIR_TEST_MOCK_GETUID", "1", 1);
+
if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) < 0)
goto cleanup;
+ unsetenv("VIR_TEST_MOCK_GETUID");
+
if (!(actual_cmd = virBufferContentAndReset(&buf))) {
int err = virBufferError(&buf);
if (err) {
--
2.21.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
