qemu: add support for sending QEMU stdout/stderr to virtlogd

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

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							\

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"

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"

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);
 

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;

src/qemu/qemu_domain.c

@@ -41,6 +41,7 @@
 #include "virstring.h"
 #include "virthreadjob.h"
 #include "viratomic.h"
+#include "logging/log_manager.h"
 
 #include "storage/storage_driver.h"
 
@@ -81,8 +82,12 @@ VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
 struct _qemuDomainLogContext {
     int refs;
     int writefd;
-    int readfd;
+    int readfd; /* Only used if manager == NULL */
     off_t pos;
+    ino_t inode; /* Only used if manager != NULL */
+    unsigned char uuid[VIR_UUID_BUFLEN]; /* Only used if manager != NULL */
+    char *name; /* Only used if manager != NULL */
+    virLogManagerPtr manager;
 };
 
 const char *
@@ -2285,47 +2290,69 @@ qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver,
     if (VIR_ALLOC(ctxt) < 0)
         goto error;
 
+    VIR_DEBUG("Context new %p stdioLogD=%d", ctxt, cfg->stdioLogD);
     ctxt->writefd = -1;
     ctxt->readfd = -1;
     virAtomicIntSet(&ctxt->refs, 1);
 
-    if (virAsprintf(&logfile, "%s/%s.log", cfg->logDir, vm->def->name) < 0)
+    if (cfg->stdioLogD) {
-        goto error;
+        ctxt->manager = virLogManagerNew(virQEMUDriverIsPrivileged(driver));
+        if (!ctxt->manager)
+            goto error;
 
-    if ((ctxt->writefd = open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) {
+        if (VIR_STRDUP(ctxt->name, vm->def->name) < 0)
-        virReportSystemError(errno, _("failed to create logfile %s"),
+            goto error;
-                             logfile);
-        goto error;
-    }
-    if (virSetCloseExec(ctxt->writefd) < 0) {
-        virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
-                             logfile);
-        goto error;
-    }
 
-    /* For unprivileged startup we must truncate the file since
+        memcpy(ctxt->uuid, vm->def->uuid, VIR_UUID_BUFLEN);
-     * we can't rely on logrotate. We don't use O_TRUNC since
-     * it is better for SELinux policy if we truncate afterwards */
-    if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START &&
-        !virQEMUDriverIsPrivileged(driver) &&
-        ftruncate(ctxt->writefd, 0) < 0) {
-        virReportSystemError(errno, _("failed to truncate %s"),
-                             logfile);
-        goto error;
-    }
 
-    if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START) {
+        ctxt->writefd = virLogManagerDomainOpenLogFile(ctxt->manager,
-        if ((ctxt->readfd = open(logfile, O_RDONLY, S_IRUSR | S_IWUSR)) < 0) {
+                                                       "qemu",
-            virReportSystemError(errno, _("failed to open logfile %s"),
+                                                       vm->def->uuid,
+                                                       vm->def->name,
+                                                       0,
+                                                       &ctxt->inode,
+                                                       &ctxt->pos);
+        if (ctxt->writefd < 0)
+            goto error;
+    } else {
+        if (virAsprintf(&logfile, "%s/%s.log", cfg->logDir, vm->def->name) < 0)
+            goto error;
+
+        if ((ctxt->writefd = open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) {
+            virReportSystemError(errno, _("failed to create logfile %s"),
                                  logfile);
             goto error;
         }
-        if (virSetCloseExec(ctxt->readfd) < 0) {
+        if (virSetCloseExec(ctxt->writefd) < 0) {
             virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
                                  logfile);
             goto error;
         }
 
+        /* For unprivileged startup we must truncate the file since
+         * we can't rely on logrotate. We don't use O_TRUNC since
+         * it is better for SELinux policy if we truncate afterwards */
+        if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START &&
+            !virQEMUDriverIsPrivileged(driver) &&
+            ftruncate(ctxt->writefd, 0) < 0) {
+            virReportSystemError(errno, _("failed to truncate %s"),
+                                 logfile);
+            goto error;
+        }
+
+        if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START) {
+            if ((ctxt->readfd = open(logfile, O_RDONLY, S_IRUSR | S_IWUSR)) < 0) {
+                virReportSystemError(errno, _("failed to open logfile %s"),
+                                     logfile);
+                goto error;
+            }
+            if (virSetCloseExec(ctxt->readfd) < 0) {
+                virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
+                                     logfile);
+                goto error;
+            }
+        }
+
         if ((ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END)) < 0) {
             virReportSystemError(errno, _("failed to seek in log file %s"),
                                  logfile);
@@ -2354,9 +2381,10 @@ int qemuDomainLogContextWrite(qemuDomainLogContextPtr ctxt,
 
     if (virVasprintf(&message, fmt, argptr) < 0)
         goto cleanup;
-    if (lseek(ctxt->writefd, 0, SEEK_END) < 0) {
+    if (!ctxt->manager &&
+        lseek(ctxt->writefd, 0, SEEK_END) < 0) {
         virReportSystemError(errno, "%s",
-                             _("Unable to see to end of domain logfile"));
+                             _("Unable to seek to end of domain logfile"));
         goto cleanup;
     }
     if (safewrite(ctxt->writefd, message, strlen(message)) < 0) {
@@ -2377,30 +2405,52 @@ int qemuDomainLogContextWrite(qemuDomainLogContextPtr ctxt,
 ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt,
                                  char **msg)
 {
+    VIR_DEBUG("Context read %p manager=%p inode=%llu pos=%llu",
+              ctxt, ctxt->manager,
+              (unsigned long long)ctxt->inode,
+              (unsigned long long)ctxt->pos);
     char *buf;
-    size_t buflen = 1024 * 128;
+    size_t buflen;
-    ssize_t got;
+    if (ctxt->manager) {
+        buf = virLogManagerDomainReadLogFile(ctxt->manager,
+                                             "qemu",
+                                             ctxt->uuid,
+                                             ctxt->name,
+                                             ctxt->inode,
+                                             ctxt->pos,
+                                             1024 * 128,
+                                             0);
+        if (!buf)
+            return -1;
+        buflen = strlen(buf);
+    } else {
+        ssize_t got;
 
-    /* Best effort jump to start of messages */
+        buflen = 1024 * 128;
-    ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET));
 
-    if (VIR_ALLOC_N(buf, buflen) < 0)
+        /* Best effort jump to start of messages */
-        return -1;
+        ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET));
 
-    got = saferead(ctxt->readfd, buf, buflen - 1);
+        if (VIR_ALLOC_N(buf, buflen) < 0)
-    if (got < 0) {
+            return -1;
-        VIR_FREE(buf);
+
-        virReportSystemError(errno, "%s",
+        got = saferead(ctxt->readfd, buf, buflen - 1);
-                             _("Unable to read from log file"));
+        if (got < 0) {
-        return -1;
+            VIR_FREE(buf);
+            virReportSystemError(errno, "%s",
+                                 _("Unable to read from log file"));
+            return -1;
+        }
+
+        buf[got] = '\0';
+
+        ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1));
+        buflen = got;
     }
 
-    buf[got] = '\0';
-
-    ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1));
     *msg = buf;
 
-    return got;
+    return buflen;
 }
 
 
@@ -2412,12 +2462,22 @@ int qemuDomainLogContextGetWriteFD(qemuDomainLogContextPtr ctxt)
 
 void qemuDomainLogContextMarkPosition(qemuDomainLogContextPtr ctxt)
 {
-    ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END);
+    if (ctxt->manager)
+        virLogManagerDomainGetLogFilePosition(ctxt->manager,
+                                              "qemu",
+                                              ctxt->uuid,
+                                              ctxt->name,
+                                              0,
+                                              &ctxt->inode,
+                                              &ctxt->pos);
+    else
+        ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END);
 }
 
 
 void qemuDomainLogContextRef(qemuDomainLogContextPtr ctxt)
 {
+    VIR_DEBUG("Context ref %p", ctxt);
     virAtomicIntInc(&ctxt->refs);
 }
 
@@ -2430,9 +2490,12 @@ void qemuDomainLogContextFree(qemuDomainLogContextPtr ctxt)
         return;
 
     lastRef = virAtomicIntDecAndTest(&ctxt->refs);
+    VIR_DEBUG("Context free %p lastref=%d", ctxt, lastRef);
     if (!lastRef)
         return;
 
+    virLogManagerFree(ctxt->manager);
+    VIR_FREE(ctxt->name);
     VIR_FORCE_CLOSE(ctxt->writefd);
     VIR_FORCE_CLOSE(ctxt->readfd);
     VIR_FREE(ctxt);

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" }