1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)

Daniel P. Berrangé posted 1 patch 7 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180212100858.4540-1-berrange@redhat.com
src/util/virlog.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
[libvirt] [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)
Posted by Daniel P. Berrangé 7 years, 3 months ago 
The fix for CVE-2018-6764 introduced a potential deadlock scenario
that gets triggered by the NSS module when virGetHostname() calls
getaddrinfo to resolve the hostname:

 #0  0x00007f6e714b57e7 in futex_wait
 #1  futex_wait_simple
 #2  __pthread_once_slow
 #3  0x00007f6e71d16e7d in virOnce
 #4  0x00007f6e71d0997c in virLogInitialize
 #5  0x00007f6e71d0a09a in virLogVMessage
 #6  0x00007f6e71d09ffd in virLogMessage
 #7  0x00007f6e71d0db22 in virObjectNew
 #8  0x00007f6e71d0dbf1 in virObjectLockableNew
 #9  0x00007f6e71d0d3e5 in virMacMapNew
 #10 0x00007f6e71cdc50a in findLease
 #11 0x00007f6e71cdcc56 in _nss_libvirt_gethostbyname4_r
 #12 0x00007f6e724631fc in gaih_inet
 #13 0x00007f6e72464697 in __GI_getaddrinfo
 #14 0x00007f6e71d19e81 in virGetHostnameImpl
 #15 0x00007f6e71d1a057 in virGetHostnameQuiet
 #16 0x00007f6e71d09936 in virLogOnceInit
 #17 0x00007f6e71d09952 in virLogOnce
 #18 0x00007f6e714b5829 in __pthread_once_slow
 #19 0x00007f6e71d16e7d in virOnce
 #20 0x00007f6e71d0997c in virLogInitialize
 #21 0x00007f6e71d0a09a in virLogVMessage
 #22 0x00007f6e71d09ffd in virLogMessage
 #23 0x00007f6e71d0db22 in virObjectNew
 #24 0x00007f6e71d0dbf1 in virObjectLockableNew
 #25 0x00007f6e71d0d3e5 in virMacMapNew
 #26 0x00007f6e71cdc50a in findLease
 #27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r
 #28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r
 #29 0x00007f6e7248f72f in __gethostbyname2_r
 #30 0x00007f6e7248f494 in gethostbyname2
 #31 0x000056348c30c36d in hosts_keys
 #32 0x000056348c30b7d2 in main

Fortunately the extra stuff virGetHostname does is totally irrelevant to
the needs of the logging code, so we can just inline a call to the
native hostname() syscall directly.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virlog.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 8f1e4800dd..4f66cc5e5c 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -64,7 +64,7 @@
 VIR_LOG_INIT("util.log");
 
 static regex_t *virLogRegex;
-static char *virLogHostname;
+static char virLogHostname[HOST_NAME_MAX+1];
 
 
 #define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}"
@@ -261,6 +261,8 @@ virLogPriorityString(virLogPriority lvl)
 static int
 virLogOnceInit(void)
 {
+    int r;
+
     if (virMutexInit(&virLogMutex) < 0)
         return -1;
 
@@ -275,8 +277,17 @@ virLogOnceInit(void)
     /* We get and remember the hostname early, because at later time
      * it might not be possible to load NSS modules via getaddrinfo()
      * (e.g. at container startup the host filesystem will not be
-     * accessible anymore. */
-    virLogHostname = virGetHostnameQuiet();
+     * accessible anymore.
+     * Must not use virGetHostname though as that causes re-entrancy
+     * problems if it triggers logging codepaths
+     */
+    r = gethostname(virLogHostname, sizeof(virLogHostname));
+    if (r == -1) {
+        ignore_value(virStrcpy(virLogHostname,
+                               "(unknown)", sizeof(virLogHostname)));
+    } else {
+        NUL_TERMINATE(virLogHostname);
+    }
 
     virLogUnlock();
     return 0;
@@ -475,9 +486,6 @@ virLogHostnameString(char **rawmsg,
 {
     char *hoststr;
 
-    if (!virLogHostname)
-        return -1;
-
     if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0)
         return -1;
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)
Posted by Michal Privoznik 7 years, 3 months ago 
On 02/12/2018 11:08 AM, Daniel P. Berrangé wrote:
> The fix for CVE-2018-6764 introduced a potential deadlock scenario
> that gets triggered by the NSS module when virGetHostname() calls
> getaddrinfo to resolve the hostname:
> 
>  #0  0x00007f6e714b57e7 in futex_wait
>  #1  futex_wait_simple
>  #2  __pthread_once_slow
>  #3  0x00007f6e71d16e7d in virOnce
>  #4  0x00007f6e71d0997c in virLogInitialize
>  #5  0x00007f6e71d0a09a in virLogVMessage
>  #6  0x00007f6e71d09ffd in virLogMessage
>  #7  0x00007f6e71d0db22 in virObjectNew
>  #8  0x00007f6e71d0dbf1 in virObjectLockableNew
>  #9  0x00007f6e71d0d3e5 in virMacMapNew
>  #10 0x00007f6e71cdc50a in findLease
>  #11 0x00007f6e71cdcc56 in _nss_libvirt_gethostbyname4_r
>  #12 0x00007f6e724631fc in gaih_inet
>  #13 0x00007f6e72464697 in __GI_getaddrinfo
>  #14 0x00007f6e71d19e81 in virGetHostnameImpl
>  #15 0x00007f6e71d1a057 in virGetHostnameQuiet
>  #16 0x00007f6e71d09936 in virLogOnceInit
>  #17 0x00007f6e71d09952 in virLogOnce
>  #18 0x00007f6e714b5829 in __pthread_once_slow
>  #19 0x00007f6e71d16e7d in virOnce
>  #20 0x00007f6e71d0997c in virLogInitialize
>  #21 0x00007f6e71d0a09a in virLogVMessage
>  #22 0x00007f6e71d09ffd in virLogMessage
>  #23 0x00007f6e71d0db22 in virObjectNew
>  #24 0x00007f6e71d0dbf1 in virObjectLockableNew
>  #25 0x00007f6e71d0d3e5 in virMacMapNew
>  #26 0x00007f6e71cdc50a in findLease
>  #27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r
>  #28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r
>  #29 0x00007f6e7248f72f in __gethostbyname2_r
>  #30 0x00007f6e7248f494 in gethostbyname2
>  #31 0x000056348c30c36d in hosts_keys
>  #32 0x000056348c30b7d2 in main
> 
> Fortunately the extra stuff virGetHostname does is totally irrelevant to
> the needs of the logging code, so we can just inline a call to the
> native hostname() syscall directly.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/util/virlog.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)
Posted by Daniel P. Berrangé 7 years, 3 months ago 
On Mon, Feb 12, 2018 at 11:52:43AM +0100, Michal Privoznik wrote:
> On 02/12/2018 11:08 AM, Daniel P. Berrangé wrote:
> > The fix for CVE-2018-6764 introduced a potential deadlock scenario
> > that gets triggered by the NSS module when virGetHostname() calls
> > getaddrinfo to resolve the hostname:
> > 
> >  #0  0x00007f6e714b57e7 in futex_wait
> >  #1  futex_wait_simple
> >  #2  __pthread_once_slow
> >  #3  0x00007f6e71d16e7d in virOnce
> >  #4  0x00007f6e71d0997c in virLogInitialize
> >  #5  0x00007f6e71d0a09a in virLogVMessage
> >  #6  0x00007f6e71d09ffd in virLogMessage
> >  #7  0x00007f6e71d0db22 in virObjectNew
> >  #8  0x00007f6e71d0dbf1 in virObjectLockableNew
> >  #9  0x00007f6e71d0d3e5 in virMacMapNew
> >  #10 0x00007f6e71cdc50a in findLease
> >  #11 0x00007f6e71cdcc56 in _nss_libvirt_gethostbyname4_r
> >  #12 0x00007f6e724631fc in gaih_inet
> >  #13 0x00007f6e72464697 in __GI_getaddrinfo
> >  #14 0x00007f6e71d19e81 in virGetHostnameImpl
> >  #15 0x00007f6e71d1a057 in virGetHostnameQuiet
> >  #16 0x00007f6e71d09936 in virLogOnceInit
> >  #17 0x00007f6e71d09952 in virLogOnce
> >  #18 0x00007f6e714b5829 in __pthread_once_slow
> >  #19 0x00007f6e71d16e7d in virOnce
> >  #20 0x00007f6e71d0997c in virLogInitialize
> >  #21 0x00007f6e71d0a09a in virLogVMessage
> >  #22 0x00007f6e71d09ffd in virLogMessage
> >  #23 0x00007f6e71d0db22 in virObjectNew
> >  #24 0x00007f6e71d0dbf1 in virObjectLockableNew
> >  #25 0x00007f6e71d0d3e5 in virMacMapNew
> >  #26 0x00007f6e71cdc50a in findLease
> >  #27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r
> >  #28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r
> >  #29 0x00007f6e7248f72f in __gethostbyname2_r
> >  #30 0x00007f6e7248f494 in gethostbyname2
> >  #31 0x000056348c30c36d in hosts_keys
> >  #32 0x000056348c30b7d2 in main
> > 
> > Fortunately the extra stuff virGetHostname does is totally irrelevant to
> > the needs of the logging code, so we can just inline a call to the
> > native hostname() syscall directly.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/util/virlog.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> ACK

FYI I'll also squash in a change to cfg.mk to avoid syntax-check failure

diff --git a/cfg.mk b/cfg.mk
index 78f805b27e..920b609172 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1183,7 +1183,7 @@ _src2=src/(util/vircommand|libvirt|lxc/lxc_controller|locking/lock_daemon|loggin
 exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
   (^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$)
 
-exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/virutil\.c$$
+exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/vir(util|log)\.c$$
 
 exclude_file_name_regexp--sc_prohibit_internal_functions = \
   ^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$


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-8c64711

© 2016 - 2025 Red Hat, Inc.