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 | 153 ++++++++++++++++++++++++++-----------
src/qemu/test_libvirtd_qemu.aug.in | 1 +
7 files changed, 145 insertions(+), 46 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 2a23b33..e35c79b 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..4fa5e8a 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 2417cb7..466b5b7 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"
@@ -80,8 +81,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 *
@@ -2260,46 +2265,68 @@ 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;
+ }
+ }
}
virObjectUnref(cfg);
@@ -2323,9 +2350,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) {
@@ -2346,29 +2374,51 @@ 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) {
- 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) {
+ 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;
}
@@ -2380,12 +2430,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);
}
@@ -2398,9 +2458,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 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