[libvirt] [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)
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
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
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 :|
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik