From: Jim Fehlig Date: Thu, 18 Nov 2021 12:03:20 -0700 Subject: libxl: Protect access to libxlLogger files hash table The hash table of log file objects in libxlLogger is not protected against concurrent access. It is possible for one thread to remove an entry while another is updating it. Add a mutex to the libxlLogger object and lock it when accessing the files hash table. Signed-off-by: Jim Fehlig Reviewed-by: Daniel P. Berrangé Reviewed-by: Ján Tomko Origin: https://gitlab.com/libvirt/libvirt/-/commit/a7a03324d86e111f81687b5315b8f296dde84340 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2034195 Bug-Debian: https://security-tracker.debian.org/tracker/CVE-2021-4147 Bug-Debian: https://bugs.debian.org/1002535 --- src/libxl/libxl_logger.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c index 93a9c76..4113d67 100644 --- a/src/libxl/libxl_logger.c +++ b/src/libxl/libxl_logger.c @@ -28,6 +28,7 @@ #include "util/virfile.h" #include "util/virhash.h" #include "util/virstring.h" +#include "util/virthread.h" #include "util/virtime.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -43,6 +44,7 @@ struct xentoollog_logger_libvirt { /* map storing the opened fds: "domid" -> FILE* */ GHashTable *files; + virMutex tableLock; FILE *defaultLogFile; }; @@ -85,7 +87,9 @@ libvirt_vmessage(xentoollog_logger *logger_in, start = start + 9; *end = '\0'; + virMutexLock(&lg->tableLock); domainLogFile = virHashLookup(lg->files, start); + virMutexUnlock(&lg->tableLock); if (domainLogFile) logFile = domainLogFile; @@ -161,6 +165,11 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel) if ((logger.defaultLogFile = fopen(path, "a")) == NULL) goto error; + if (virMutexInit(&logger.tableLock) < 0) { + VIR_FORCE_FCLOSE(logger.defaultLogFile); + goto error; + } + logger_out = XTL_NEW_LOGGER(libvirt, logger); cleanup: @@ -179,6 +188,7 @@ libxlLoggerFree(libxlLoggerPtr logger) if (logger->defaultLogFile) VIR_FORCE_FCLOSE(logger->defaultLogFile); virHashFree(logger->files); + virMutexDestroy(&logger->tableLock); xtl_logger_destroy(xtl_logger); } @@ -200,7 +210,9 @@ libxlLoggerOpenFile(libxlLoggerPtr logger, path, g_strerror(errno)); goto cleanup; } + virMutexLock(&logger->tableLock); ignore_value(virHashAddEntry(logger->files, domidstr, logFile)); + virMutexUnlock(&logger->tableLock); /* domain_config is non NULL only when starting a new domain */ if (domain_config) { @@ -219,7 +231,9 @@ libxlLoggerCloseFile(libxlLoggerPtr logger, int id) char *domidstr = NULL; domidstr = g_strdup_printf("%d", id); + virMutexLock(&logger->tableLock); ignore_value(virHashRemoveEntry(logger->files, domidstr)); + virMutexUnlock(&logger->tableLock); VIR_FREE(domidstr); }