From a7a03324d86e111f81687b5315b8f296dde84340 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Thu, 18 Nov 2021 12:03:20 -0700
Subject: [PATCH] libxl: Protect access to libxlLogger files hash table
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

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 <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 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 d176c2a7c2..44ee5a95da 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;
 
@@ -155,6 +159,11 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)
     if ((logger.defaultLogFile = fopen(path, "a")) == NULL)
         return NULL;
 
+    if (virMutexInit(&logger.tableLock) < 0) {
+        VIR_FORCE_FCLOSE(logger.defaultLogFile);
+        return NULL;
+    }
+
     logger.files = virHashNew(libxlLoggerFileFree);
 
     return XTL_NEW_LOGGER(libvirt, logger);
@@ -167,6 +176,7 @@ libxlLoggerFree(libxlLogger *logger)
     if (logger->defaultLogFile)
         VIR_FORCE_FCLOSE(logger->defaultLogFile);
     g_clear_pointer(&logger->files, g_hash_table_unref);
+    virMutexDestroy(&logger->tableLock);
     xtl_logger_destroy(xtl_logger);
 }
 
@@ -188,7 +198,9 @@ libxlLoggerOpenFile(libxlLogger *logger,
                  path, g_strerror(errno));
         return;
     }
+    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) {
@@ -203,5 +215,7 @@ libxlLoggerCloseFile(libxlLogger *logger, int id)
     g_autofree char *domidstr = NULL;
     domidstr = g_strdup_printf("%d", id);
 
+    virMutexLock(&logger->tableLock);
     ignore_value(virHashRemoveEntry(logger->files, domidstr));
+    virMutexUnlock(&logger->tableLock);
 }
-- 
2.39.5