Currently the QEMU stdout/stderr streams are written directly to
a regular file (eg /var/log/libvirt/qemu/$GUEST.log). While those
can be rotated by logrotate (using copytruncate option) this is
not very efficient. It also leaves open a window of opportunity
for a compromised/broken QEMU to DOS the host filesystem by
writing lots of text to stdout/stderr.
This makes it possible to connect the stdout/stderr file handles
to a pipe that is provided by virtlogd. The virtlogd daemon will
read from this pipe and write data to the log file, performing
file rotation whenever a pre-determined size limit is reached.
Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
cfg.mk | 2 +-
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 15 +++++++++++++
src/qemu/qemu_conf.c | 18 ++++++++++++++++
src/qemu/qemu_conf.h | 1 +
src/qemu/qemu_domain.c | 43 +++++++++++++++++++++++++++++++-------
src/qemu/qemu_process.c | 42 +++++++++++++++++++++----------------
src/qemu/test_libvirtd_qemu.aug.in | 1 +
8 files changed, 96 insertions(+), 27 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index cf18a84..f3f43f2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -775,7 +775,7 @@ sc_prohibit_gettext_markup:
# lower-level code must not include higher-level headers.
cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.))
cross_dirs_re=($(subst / ,/|,$(cross_dirs)))
-mid_dirs=access|conf|cpu|locking|network|node_device|rpc|security|storage
+mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storage
sc_prohibit_cross_inclusion:
@for dir in $(cross_dirs); do \
case $$dir in \
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 62951da..b6f6dc4 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -71,6 +71,7 @@ module Libvirtd_qemu =
| bool_entry "set_process_name"
| int_entry "max_processes"
| int_entry "max_files"
+ | str_entry "stdio_handler"
let device_entry = bool_entry "mac_filter"
| bool_entry "relaxed_acs_check"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 1c589a2..084323f 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -515,3 +515,18 @@
# "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",
# "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd"
#]
+
+# The backend to use for handling stdout/stderr output from
+# QEMU processes.
+#
+# 'file': QEMU writes directly to a plain file. This is the
+# historical default, but allows QEMU to inflict a
+# denial of service attack on the host by exhausting
+# filesystem space
+#
+# 'logd': QEMU writes to a pipe provided by virtlogd daemon.
+# This is the current default, providing protection
+# against denial of service by performing log file
+# rollover when a size limit is hit.
+#
+#stdio_handler = logd
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 658a50e..14683f5 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -454,6 +454,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
virConfValuePtr p;
int ret = -1;
size_t i;
+ char *stdioHandler = NULL;
/* Just check the file is readable before opening it, otherwise
* libvirt emits an error.
@@ -781,6 +782,23 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_VALUE_ULONG("max_files", cfg->maxFiles);
GET_VALUE_STR("lock_manager", cfg->lockManagerName);
+ GET_VALUE_STR("stdio_handler", stdioHandler);
+ if (stdioHandler) {
+ if (STREQ(stdioHandler, "logd")) {
+ cfg->stdioLogD = true;
+ } else if (STREQ(stdioHandler, "file")) {
+ cfg->stdioLogD = false;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown stdio handler %s"),
+ stdioHandler);
+ VIR_FREE(stdioHandler);
+ goto cleanup;
+ }
+ VIR_FREE(stdioHandler);
+ } else {
+ cfg->stdioLogD = true;
+ }
GET_VALUE_ULONG("max_queued", cfg->maxQueuedJobs);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index ed9cd46..4b33770 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -173,6 +173,7 @@ struct _virQEMUDriverConfig {
int migrationPortMax;
bool logTimestamp;
+ bool stdioLogD;
/* Pairs of loader:nvram paths. The list is @nloader items long */
char **loader;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 890d8ed..c9dbdde 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,6 +40,7 @@
#include "virstoragefile.h"
#include "virstring.h"
#include "virthreadjob.h"
+#include "logging/log_manager.h"
#include "storage/storage_driver.h"
@@ -2275,22 +2276,48 @@ qemuDomainOpenLogHelper(virQEMUDriverConfigPtr cfg,
}
+static int
+qemuDomainCreateLogdFile(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+ int fd = -1;
+ virLogManagerPtr mgr = NULL;
+
+ if (!(mgr = virLogManagerNew(driver->privileged)))
+ goto cleanup;
+
+ fd = virLogManagerDomainOpenLogFile(mgr,
+ "qemu",
+ vm->def->uuid,
+ vm->def->name, 0);
+
+ cleanup:
+ virLogManagerFree(mgr);
+ return fd;
+}
+
+
int
qemuDomainCreateLog(virQEMUDriverPtr driver, virDomainObjPtr vm,
bool append)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- int oflags;
int ret;
- oflags = O_CREAT | O_WRONLY;
- /* Only logrotate files in /var/log, so only append if running privileged */
- if (virQEMUDriverIsPrivileged(driver) || append)
- oflags |= O_APPEND;
- else
- oflags |= O_TRUNC;
+ if (cfg->stdioLogD) {
+ /* virtlogd always operates in append mode */
+ ret = qemuDomainCreateLogdFile(driver, vm);
+ } else {
+ int oflags;
+ oflags = O_CREAT | O_WRONLY;
+ /* Only logrotate files in /var/log, so only append if running privileged */
+ if (virQEMUDriverIsPrivileged(driver) || append)
+ oflags |= O_APPEND;
+ else
+ oflags |= O_TRUNC;
- ret = qemuDomainOpenLogHelper(cfg, vm, oflags, S_IRUSR | S_IWUSR);
+ ret = qemuDomainOpenLogHelper(cfg, vm, oflags, S_IRUSR | S_IWUSR);
+ }
virObjectUnref(cfg);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f744419..e72ca20 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4774,7 +4774,10 @@ int qemuProcessStart(virConnectPtr conn,
qemuDomainObjCheckTaint(driver, vm, logfile);
- if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
+ /* When using logd, the logfile FD is a pipe which is
+ * not seekable... */
+ if (!cfg->stdioLogD &&
+ (pos = lseek(logfile, 0, SEEK_END)) < 0)
VIR_WARN("Unable to seek to end of logfile: %s",
virStrerror(errno, ebuf, sizeof(ebuf)));
@@ -5201,26 +5204,29 @@ void qemuProcessStop(virQEMUDriverPtr driver,
/* Wake up anything waiting on domain condition */
virDomainObjBroadcast(vm);
- if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) {
- /* To not break the normal domain shutdown process, skip the
- * timestamp log writing if failed on opening log file. */
- VIR_WARN("Unable to open logfile: %s",
- virStrerror(errno, ebuf, sizeof(ebuf)));
- } else {
- if ((timestamp = virTimeStringNow()) != NULL) {
- if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 ||
- safewrite(logfile, SHUTDOWN_POSTFIX,
- strlen(SHUTDOWN_POSTFIX)) < 0) {
- VIR_WARN("Unable to write timestamp to logfile: %s",
- virStrerror(errno, ebuf, sizeof(ebuf)));
+ /* We let virtlogd put stop markers in */
+ if (!cfg->stdioLogD) {
+ if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) {
+ /* To not break the normal domain shutdown process, skip the
+ * timestamp log writing if failed on opening log file. */
+ VIR_WARN("Unable to open logfile: %s",
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ } else {
+ if ((timestamp = virTimeStringNow()) != NULL) {
+ if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 ||
+ safewrite(logfile, SHUTDOWN_POSTFIX,
+ strlen(SHUTDOWN_POSTFIX)) < 0) {
+ VIR_WARN("Unable to write timestamp to logfile: %s",
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ }
+
+ VIR_FREE(timestamp);
}
- VIR_FREE(timestamp);
+ if (VIR_CLOSE(logfile) < 0)
+ VIR_WARN("Unable to close logfile: %s",
+ virStrerror(errno, ebuf, sizeof(ebuf)));
}
-
- if (VIR_CLOSE(logfile) < 0)
- VIR_WARN("Unable to close logfile: %s",
- virStrerror(errno, ebuf, sizeof(ebuf)));
}
/* Clear network bandwidth */
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index fc4935b..8bec743 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -78,3 +78,4 @@ module Test_libvirtd_qemu =
{ "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" }
{ "2" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" }
}
+{ "stdio_handler" = "logd" }
--
2.5.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list