From 0d968ad715475a1660779bcdd2c5b38ad63db4cf Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 3 Nov 2015 11:13:25 +0000 Subject: [PATCH] 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 --- 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 | 155 ++++++++++++++++++++--------- src/qemu/test_libvirtd_qemu.aug.in | 1 + 7 files changed, 146 insertions(+), 47 deletions(-) diff --git a/cfg.mk b/cfg.mk index 8530e1789..c58e38837 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 62951daa2..b6f6dc49f 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 1c589a232..4fa5e8ad4 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 658a50e3b..14683f5c8 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 ed9cd4669..4b33770bd 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 a6a0eedae..ed212456a 100644 --- a/src/qemu/qemu_domain.c +++ b/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) - goto error; + if (cfg->stdioLogD) { + 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) { - virReportSystemError(errno, _("failed to create logfile %s"), - logfile); - goto error; - } - if (virSetCloseExec(ctxt->writefd) < 0) { - virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), - logfile); - goto error; - } + if (VIR_STRDUP(ctxt->name, vm->def->name) < 0) + 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; - } + memcpy(ctxt->uuid, vm->def->uuid, VIR_UUID_BUFLEN); - 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"), + ctxt->writefd = virLogManagerDomainOpenLogFile(ctxt->manager, + "qemu", + 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; - ssize_t got; + size_t buflen; + 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 */ - ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET)); + buflen = 1024 * 128; - if (VIR_ALLOC_N(buf, buflen) < 0) - return -1; + /* Best effort jump to start of messages */ + ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET)); - got = saferead(ctxt->readfd, buf, buflen - 1); - if (got < 0) { - VIR_FREE(buf); - virReportSystemError(errno, "%s", - _("Unable to read from log file")); - return -1; - } + if (VIR_ALLOC_N(buf, buflen) < 0) + return -1; - buf[got] = '\0'; + got = saferead(ctxt->readfd, buf, buflen - 1); + if (got < 0) { + 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; + } - 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); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index fc4935b75..8bec7437c 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.39.5