1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] tests: Add getuid() to virnetdevbandwidthmock

Andrea Bolognani posted 1 patch 4 years, 9 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190708160310.12376-1-abologna@redhat.com
tests/virnetdevbandwidthmock.c | 5 +++++
1 file changed, 5 insertions(+)
[libvirt] [PATCH] tests: Add getuid() to virnetdevbandwidthmock
Posted by Andrea Bolognani 4 years, 9 months ago 
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
  #12 0x00007ffff7fe437a in call_init (l=<optimized out>, argc=argc@entry=1, argv=argv@entry=0x7fffffffe778, env=env@entry=0x7fffffffe788) at dl-init.c:72
  #13 0x00007ffff7fe4476 in call_init (env=0x7fffffffe788, argv=0x7fffffffe778, argc=1, l=<optimized out>) at dl-init.c:30
  #14 _dl_init (main_map=0x7ffff7ffe190, argc=1, argv=0x7fffffffe778, env=0x7fffffffe788) at dl-init.c:119
  #15 0x00007ffff7fd60ca in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
  #16 0x0000000000000001 in ?? ()
  #17 0x00007fffffffea26 in ?? ()
  #18 0x0000000000000000 in ?? ()

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 tests/virnetdevbandwidthmock.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/virnetdevbandwidthmock.c b/tests/virnetdevbandwidthmock.c
index cb48933447..f0c6b22c5f 100644
--- a/tests/virnetdevbandwidthmock.c
+++ b/tests/virnetdevbandwidthmock.c
@@ -24,3 +24,8 @@ uid_t geteuid(void)
 {
     return 0;
 }
+
+uid_t getuid(void)
+{
+    return 0;
+}
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Add getuid() to virnetdevbandwidthmock
Posted by Daniel P. Berrangé 4 years, 9 months ago 
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.

>   #12 0x00007ffff7fe437a in call_init (l=<optimized out>, argc=argc@entry=1, argv=argv@entry=0x7fffffffe778, env=env@entry=0x7fffffffe788) at dl-init.c:72
>   #13 0x00007ffff7fe4476 in call_init (env=0x7fffffffe788, argv=0x7fffffffe778, argc=1, l=<optimized out>) at dl-init.c:30
>   #14 _dl_init (main_map=0x7ffff7ffe190, argc=1, argv=0x7fffffffe778, env=0x7fffffffe788) at dl-init.c:119
>   #15 0x00007ffff7fd60ca in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
>   #16 0x0000000000000001 in ?? ()
>   #17 0x00007fffffffea26 in ?? ()
>   #18 0x0000000000000000 in ?? ()
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  tests/virnetdevbandwidthmock.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/virnetdevbandwidthmock.c b/tests/virnetdevbandwidthmock.c
> index cb48933447..f0c6b22c5f 100644
> --- a/tests/virnetdevbandwidthmock.c
> +++ b/tests/virnetdevbandwidthmock.c
> @@ -24,3 +24,8 @@ uid_t geteuid(void)
>  {
>      return 0;
>  }
> +
> +uid_t getuid(void)
> +{
> +    return 0;
> +}
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Add getuid() to virnetdevbandwidthmock
Posted by Andrea Bolognani 4 years, 9 months ago 
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Add getuid() to virnetdevbandwidthmock
Posted by Daniel P. Berrangé 4 years, 9 months ago 
On Tue, Jul 09, 2019 at 02:45:54PM +0200, Andrea Bolognani wrote:
> 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:-)

NO, you got what I was thinking, but if it doesn't work, lets not
waste more time on it.

Go for your original quick fix:

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.