[libvirt] [PATCH 5/5] qemu: add support for sending QEMU stdout/stderr to virtlogd

Daniel P. Berrange berrange at redhat.com
Tue Nov 3 16:04:24 UTC 2015


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 at redhat.com>
---
 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




More information about the libvir-list mailing list