Skip to content

Commit

Permalink
log: fix deadlock obtaining hostname (related CVE-2018-6764)
Browse files Browse the repository at this point in the history
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
 libvirt#19 0x00007f6e71d16e7d in virOnce
 libvirt#20 0x00007f6e71d0997c in virLogInitialize
 libvirt#21 0x00007f6e71d0a09a in virLogVMessage
 libvirt#22 0x00007f6e71d09ffd in virLogMessage
 libvirt#23 0x00007f6e71d0db22 in virObjectNew
 libvirt#24 0x00007f6e71d0dbf1 in virObjectLockableNew
 libvirt#25 0x00007f6e71d0d3e5 in virMacMapNew
 libvirt#26 0x00007f6e71cdc50a in findLease
 libvirt#27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r
 libvirt#28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r
 libvirt#29 0x00007f6e7248f72f in __gethostbyname2_r
 libvirt#30 0x00007f6e7248f494 in gethostbyname2
 libvirt#31 0x000056348c30c36d in hosts_keys
 libvirt#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é <[email protected]>
  • Loading branch information
berrange committed Feb 12, 2018
1 parent 42fd5a5 commit c2dc669
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
2 changes: 1 addition & 1 deletion cfg.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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)$$
Expand Down
20 changes: 14 additions & 6 deletions src/util/virlog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -261,6 +261,8 @@ virLogPriorityString(virLogPriority lvl)
static int
virLogOnceInit(void)
{
int r;

if (virMutexInit(&virLogMutex) < 0)
return -1;

Expand All @@ -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;
Expand Down Expand Up @@ -475,9 +486,6 @@ virLogHostnameString(char **rawmsg,
{
char *hoststr;

if (!virLogHostname)
return -1;

if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0)
return -1;

Expand Down

0 comments on commit c2dc669

Please sign in to comment.