CVE-2018-5748: resource exhaustion via qemuMonitorIORead() (bz #1535785)

CVE-2018-6764: code injection via libvirt_lxc (bz #1542815)
Fix hotplug disk failure (bz #1540872)
This commit is contained in:
Cole Robinson 2018-02-13 14:26:06 -05:00
parent c23de3143a
commit 7294ce1ae2
7 changed files with 371 additions and 1 deletions

View File

@ -0,0 +1,36 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 20 Dec 2017 12:58:36 +0100
Subject: [PATCH] util: probe: Add quiet versions of the "PROBE" macro
PROBE macro adds a logging entry, when used in places seeing a lot of
traffic this can cause a significant slowdown.
(cherry picked from commit f06e488d5484031a76e7ed231c8fef8fa1181d2c)
---
src/util/virprobe.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/util/virprobe.h b/src/util/virprobe.h
index 7565954af..bd8c32964 100644
--- a/src/util/virprobe.h
+++ b/src/util/virprobe.h
@@ -90,11 +90,19 @@
PROBE_EXPAND(LIBVIRT_ ## NAME, \
VIR_ADD_CASTS(__VA_ARGS__)); \
}
+
+# define PROBE_QUIET(NAME, FMT, ...) \
+ if (LIBVIRT_ ## NAME ## _ENABLED()) { \
+ PROBE_EXPAND(LIBVIRT_ ## NAME, \
+ VIR_ADD_CASTS(__VA_ARGS__)); \
+ }
# else
# define PROBE(NAME, FMT, ...) \
VIR_INFO_INT(&virLogSelf, \
__FILE__, __LINE__, __func__, \
#NAME ": " FMT, __VA_ARGS__);
+
+# define PROBE_QUIET(NAME, FMT, ...)
# endif
#endif /* __VIR_PROBE_H__ */

View File

@ -0,0 +1,49 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 20 Dec 2017 13:09:07 +0100
Subject: [PATCH] qemu: monitor: Decrease logging verbosity
The PROBE macro used in qemuMonitorIOProcess and the VIR_DEBUG message
in qemuMonitorJSONIOProcess create a lot of logging churn when debug
logging is enabled during monitor communication.
The messages logged from the PROBE macro are rather useless since they
are reporting the partial state of receiving the reply from qemu. The
actual full reply is still logged in qemuMonitorJSONIOProcessLine once
the full message is received.
(cherry picked from commit f10bb3347b43d900ff361cda5fe1996782284991)
---
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_monitor_json.c | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19082d8bf..3def28852 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -434,8 +434,8 @@ qemuMonitorIOProcess(qemuMonitorPtr mon)
# endif
#endif
- PROBE(QEMU_MONITOR_IO_PROCESS,
- "mon=%p buf=%s len=%zu", mon, mon->buffer, mon->bufferOffset);
+ PROBE_QUIET(QEMU_MONITOR_IO_PROCESS, "mon=%p buf=%s len=%zu",
+ mon, mon->buffer, mon->bufferOffset);
if (mon->json)
len = qemuMonitorJSONIOProcess(mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index df5fb7c8f..461aae089 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -259,7 +259,10 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon,
}
}
+#if DEBUG_IO
VIR_DEBUG("Total used %d bytes out of %zd available in buffer", used, len);
+#endif
+
return used;
}

View File

@ -0,0 +1,63 @@
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sat, 27 Jan 2018 23:43:58 +0100
Subject: [PATCH] virlog: determine the hostname on startup CVE-2018-6764
At later point it might not be possible or even safe to use getaddrinfo(). It
can in turn result in a load of NSS module.
Notably, on a LXC container startup we may find ourselves with the guest
filesystem already having replaced the host one. Loading a NSS module
from the guest tree would allow a malicous guest to escape the
confinement of its container environment because libvirt will not yet
have locked it down.
(cherry picked from commit 759b4d1b0fe5f4d84d98b99153dfa7ac289dd167)
---
src/util/virlog.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index d45a451a7..05e0e199e 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -64,6 +64,7 @@
VIR_LOG_INIT("util.log");
static regex_t *virLogRegex;
+static char *virLogHostname;
#define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}"
@@ -271,6 +272,12 @@ virLogOnceInit(void)
VIR_FREE(virLogRegex);
}
+ /* 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();
+
virLogUnlock();
return 0;
}
@@ -466,17 +473,14 @@ static int
virLogHostnameString(char **rawmsg,
char **msg)
{
- char *hostname = virGetHostnameQuiet();
char *hoststr;
- if (!hostname)
+ if (!virLogHostname)
return -1;
- if (virAsprintfQuiet(&hoststr, "hostname: %s", hostname) < 0) {
- VIR_FREE(hostname);
+ if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0) {
return -1;
}
- VIR_FREE(hostname);
if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) {
VIR_FREE(hoststr);

View File

@ -0,0 +1,27 @@
From: Andrea Bolognani <abologna@redhat.com>
Date: Wed, 7 Feb 2018 14:39:18 +0100
Subject: [PATCH] util: Fix syntax-check
Broken by 759b4d1b0fe5f4d84d98b99153dfa7ac289dd167.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
(cherry picked from commit 6ce3acc129bfdbe7fd02bcb8bbe8af6d13903684)
---
src/util/virlog.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 05e0e199e..056b53cda 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -478,9 +478,8 @@ virLogHostnameString(char **rawmsg,
if (!virLogHostname)
return -1;
- if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0) {
+ if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0)
return -1;
- }
if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) {
VIR_FREE(hoststr);

View File

@ -0,0 +1,121 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 12 Feb 2018 10:03:08 +0000
Subject: [PATCH] log: fix deadlock obtaining hostname (related CVE-2018-6764)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
(cherry picked from commit c2dc6698c88fb591639e542c8ecb0076c54f3dfb)
---
cfg.mk | 2 +-
src/util/virlog.c | 20 ++++++++++++++------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 56cb14bd9..a4131592c 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1158,7 +1158,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)$$
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 056b53cda..f76fc2caf 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;

View File

@ -0,0 +1,59 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 4 Jan 2018 11:11:53 +0100
Subject: [PATCH] qemuDomainAttachDeviceMknodHelper: Remove symlink before
creating it
https://bugzilla.redhat.com/show_bug.cgi?id=1528502
So imagine you have /dev/blah symlink which points to /dev/sda.
You attach /dev/blah as disk to your domain. Libvirt correctly
creates the /dev/blah -> /dev/sda symlink in the qemu namespace.
However, then you detach the disk, change the symlink so that it
points to /dev/sdb and tries to attach the disk again. This time,
however, the attach fails (well, qemu attaches wrong disk)
because the code assumes that symlinks don't change. Well they
do.
This is inspired by test fix written by Eduardo Habkost.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
(cherry picked from commit db98e7f67ea0d7699410f514f01947cef5128a6c)
---
src/qemu/qemu_domain.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 42d17c1b0..e0f4aaafa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8864,13 +8864,23 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
if (isLink) {
VIR_DEBUG("Creating symlink %s -> %s", data->file, data->target);
+
+ /* First, unlink the symlink target. Symlinks change and
+ * therefore we have no guarantees that pre-existing
+ * symlink is still valid. */
+ if (unlink(data->file) < 0 &&
+ errno != ENOENT) {
+ virReportSystemError(errno,
+ _("Unable to remove symlink %s"),
+ data->file);
+ goto cleanup;
+ }
+
if (symlink(data->target, data->file) < 0) {
- if (errno != EEXIST) {
- virReportSystemError(errno,
- _("Unable to create symlink %s"),
- data->target);
- goto cleanup;
- }
+ virReportSystemError(errno,
+ _("Unable to create symlink %s (pointing to %s)"),
+ data->file, data->target);
+ goto cleanup;
} else {
delDevice = true;
}

View File

@ -240,7 +240,7 @@
Summary: Library providing a simple virtualization API
Name: libvirt
Version: 3.7.0
Release: 3%{?dist}%{?extra_release}
Release: 4%{?dist}%{?extra_release}
License: LGPLv2+
Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
@ -268,6 +268,16 @@ Patch0009: 0009-qemu-Disallow-pivot-of-shared-disks-to-unsupported-s.patch
Patch0010: 0010-qemu-caps-Add-capability-for-share-rw-disk-option.patch
Patch0011: 0011-qemu-command-Mark-shared-disks-as-such-in-qemu.patch
# CVE-2018-5748: resource exhaustion via qemuMonitorIORead() (bz #1535785)
Patch0101: 0101-util-probe-Add-quiet-versions-of-the-PROBE-macro.patch
Patch0102: 0102-qemu-monitor-Decrease-logging-verbosity.patch
# CVE-2018-6764: code injection via libvirt_lxc (bz #1542815)
Patch0103: 0103-virlog-determine-the-hostname-on-startup-CVE-2018-67.patch
Patch0104: 0104-util-Fix-syntax-check.patch
Patch0105: 0105-log-fix-deadlock-obtaining-hostname-related-CVE-2018.patch
# Fix hotplug disk failure (bz #1540872)
Patch0106: 0106-qemuDomainAttachDeviceMknodHelper-Remove-symlink-bef.patch
Requires: libvirt-daemon = %{version}-%{release}
Requires: libvirt-daemon-config-network = %{version}-%{release}
Requires: libvirt-daemon-config-nwfilter = %{version}-%{release}
@ -2138,6 +2148,11 @@ exit 0
%changelog
* Tue Feb 13 2018 Cole Robinson <crobinso@redhat.com> - 3.7.0-4
- CVE-2018-5748: resource exhaustion via qemuMonitorIORead() (bz #1535785)
- CVE-2018-6764: code injection via libvirt_lxc (bz #1542815)
- Fix hotplug disk failure (bz #1540872)
* Mon Dec 04 2017 Cole Robinson <crobinso@redhat.com> - 3.7.0-3
- CVE-2017-1000256: libvirt: TLS certificate verification disabled for
clients (bz #1503687)