util: Fix race condition in virFileIsSharedFSType

virFileIsSharedFSType could end up calling statfs on a path that no
longer exists and return an error. If this happens for a path on a
shared filesystem, the caller may incorrectly consider the path as
non-shared.

Specifically, when starting a domain with TPM enabled and deciding
whether its vTPM state is stored on a shared storage, the race could
cause qemuTPMEmulatorBuildCommand to consider the state to be
non-shared. This means swtpm would be started without --migration even
when the state is actually stored on a shared storage and any attempt to
migrate such domain would fail with

    Operation not supported: the running swtpm does not support
    migration with shared storage

In fact, any caller of virFileGetExistingParent contained an inherent
TOCTOU race condition as the existing parent of a given path return by
virFileGetExistingParent may no longer exist at the time the caller
wants to check it.

This patch introduces a new virFileCheckParents API which is almost
identical to virFileGetExistingParent, but uses a supplied callback to
check each path. This new API is used in virFileIsSharedFSType to avoid
the race. The old function will later be completely removed once all
callers are switched to the new one.

Fixes: 05526b50909ff50c16e13a0b5580d41de74e3d59
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

diff --git a/src/util/virfile.c b/src/util/virfile.c
index f195d02e29fdd0b431068888fa32e1e59bd1fdaf..b0f4041df470432c4f8310ecc2c150222ae1fa73 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3445,6 +3445,63 @@ virFileRemoveLastComponent(char *path)
 }
 
 
+/* Check callback for virFileCheckParents */
+typedef bool (*virFileCheckParentsCallback)(const char *dirpath,
+                                            void *opaque);
+
+/**
+ * virFileCheckParents:
+ * @path: path to check
+ * @parent: where to store the closest parent satisfying the check
+ * @check: callback called on parent paths
+ * @opaque: data for the @check callback
+ *
+ * Calls @check on the @path and its parent paths until it returns true or a
+ * root directory is reached. When @check returns true, the @parent (if
+ * non-NULL) will be set to a copy of the corresponding path. The caller is
+ * responsible for freeing it.
+ *
+ * Returns  0 on success (@parent set),
+ *         -1 on invalid input,
+ *         -2 when no path (including "/") satisfies the @check.
+ */
+static int
+virFileCheckParents(const char *path,
+                    char **parent,
+                    virFileCheckParentsCallback check,
+                    void *opaque)
+{
+    g_autofree char *dirpath = g_strdup(path);
+    char *p = NULL;
+    bool checkOK;
+
+    checkOK = check(dirpath, opaque);
+
+    while (!checkOK && p != dirpath) {
+        if (!(p = strrchr(dirpath, G_DIR_SEPARATOR))) {
+            virReportSystemError(EINVAL,
+                                 _("Invalid absolute path '%1$s'"), path);
+            return -1;
+        }
+
+        if (p == dirpath)
+            *(p + 1) = '\0';
+        else
+            *p = '\0';
+
+        checkOK = check(dirpath, opaque);
+    }
+
+    if (!checkOK)
+        return -2;
+
+    if (parent)
+        *parent = g_steal_pointer(&dirpath);
+
+    return 0;
+}
+
+
 static char *
 virFileGetExistingParent(const char *path)
 {
@@ -3599,6 +3656,14 @@ static const struct virFileSharedFsData virFileSharedFs[] = {
 };
 
 
+static bool
+virFileCheckParentsStatFS(const char *path,
+                          void *opaque)
+{
+    return statfs(path, (struct statfs *) opaque) == 0;
+}
+
+
 int
 virFileIsSharedFSType(const char *path,
                       unsigned int fstypes)
@@ -3607,11 +3672,13 @@ virFileIsSharedFSType(const char *path,
     struct statfs sb;
     long long f_type = 0;
     size_t i;
+    int rc;
 
-    if (!(dirpath = virFileGetExistingParent(path)))
+    if ((rc = virFileCheckParents(path, &dirpath,
+                                  virFileCheckParentsStatFS, &sb)) == -1)
         return -1;
 
-    if (statfs(dirpath, &sb) < 0) {
+    if (rc != 0) {
         virReportSystemError(errno,
                              _("cannot determine filesystem for '%1$s'"),
                              path);