From 9372343f7cbe5c8f1bf0a0eb87ad1ddcdcd15503 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Fri, 11 Sep 2015 14:15:50 +0100
Subject: [PATCH] xen: fix race in refresh of config cache

The xenXMConfigCacheRefresh method scans /etc/xen and loads
all config files it finds. It then scans its internal hash
table and purges any (previously) loaded config files whose
refresh timestamp does not match the timestamp recorded at
the start of xenXMConfigCacheRefresh(). There is unfortunately
a subtle flaw in this, because if loading the config files
takes longer than 1 second, some of the config files will
have a refresh timestamp that is 1 or more seconds different
(newer) than is checked for. So we immediately purge a bunch
of valid config files we just loaded.

To avoid this flaw, we must pass the timestamp we record at
the start of xenXMConfigCacheRefresh() into the
xenXMConfigCacheAddFile() method, instead of letting the
latter call time(NULL) again.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 427067f7ed880abb053ffe8f5b904b0be4af8195)
---
 src/xen/xen_inotify.c | 10 ++++++----
 src/xen/xm_internal.c |  7 +++----
 src/xen/xm_internal.h |  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index cd1e2dfba9..d81a35d427 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -217,11 +217,11 @@ xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, const char *fname)
 }
 
 static int
-xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname)
+xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname, time_t now)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     return priv->useXenConfigCache ?
-        xenXMConfigCacheAddFile(conn, fname) :
+        xenXMConfigCacheAddFile(conn, fname, now) :
         xenInotifyXendDomainsDirAddEntry(conn, fname);
 }
 
@@ -238,6 +238,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
     char *tmp, *name;
     virConnectPtr conn = data;
     xenUnifiedPrivatePtr priv = NULL;
+    time_t now = time(NULL);
 
     VIR_DEBUG("got inotify event");
 
@@ -300,7 +301,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
             }
         } else if (e->mask & (IN_CREATE | IN_CLOSE_WRITE | IN_MOVED_TO)) {
             virObjectEventPtr event;
-            if (xenInotifyAddDomainConfigInfo(conn, fname) < 0) {
+            if (xenInotifyAddDomainConfigInfo(conn, fname, now) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                "%s", _("Error adding file to config cache"));
                 goto cleanup;
@@ -344,6 +345,7 @@ xenInotifyOpen(virConnectPtr conn,
     char *path;
     xenUnifiedPrivatePtr priv = conn->privateData;
     int direrr;
+    time_t now = time(NULL);
 
     virCheckFlags(VIR_CONNECT_RO, -1);
 
@@ -374,7 +376,7 @@ xenInotifyOpen(virConnectPtr conn,
                 return -1;
             }
 
-            if (xenInotifyAddDomainConfigInfo(conn, path) < 0) {
+            if (xenInotifyAddDomainConfigInfo(conn, path, now) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                "%s", _("Error adding file to config list"));
                 closedir(dh);
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 59b1cd4afa..07b9eb46e6 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -191,15 +191,14 @@ xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename)
  * calling this function
  */
 int
-xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
+xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename, time_t now)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     xenXMConfCachePtr entry;
     struct stat st;
     int newborn = 0;
-    time_t now = time(NULL);
 
-    VIR_DEBUG("Adding file %s", filename);
+    VIR_DEBUG("Adding file %s %lld", filename, (long long)now);
 
     /* Get modified time */
     if ((stat(filename, &st) < 0)) {
@@ -373,7 +372,7 @@ xenXMConfigCacheRefresh(virConnectPtr conn)
 
         /* If we already have a matching entry and it is not
            modified, then carry on to next one*/
-        if (xenXMConfigCacheAddFile(conn, path) < 0) {
+        if (xenXMConfigCacheAddFile(conn, path, now) < 0) {
             /* Ignoring errors, since a lot of stuff goes wrong in /etc/xen */
         }
 
diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
index 25b4fd5651..0246895d89 100644
--- a/src/xen/xm_internal.h
+++ b/src/xen/xm_internal.h
@@ -31,7 +31,7 @@
 # include "domain_conf.h"
 
 int xenXMConfigCacheRefresh (virConnectPtr conn);
-int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename);
+int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename, time_t now);
 int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename);
 
 int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags);
-- 
GitLab