[PATCH] xen: fix race in refresh of config cache

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 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 cd1e2df..d81a35d 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 59b1cd4..07b9eb4 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 25b4fd5..0246895 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);
-- 
2.4.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list