81d5674f22
- tests: add test for a single per-device smmuv3 (RHEL-74200) - qemu: Use pci_bus to identify multi-smmuv3 model (RHEL-74200) - qemu: tpm: Account for possible migration without actually sharing storage (RHEL-132534) - tests: Test virFileIsSharedFSOverride (RHEL-102925) - util: Fix race condition in virFileIsSharedFSType (RHEL-102925) - util: Fix race condition in virFileIsSharedFSOverride (RHEL-102925) - util: Rework virFileIsSharedFSOverride using virFileCheckParents (RHEL-102925) Resolves: RHEL-102925, RHEL-132534, RHEL-74200
145 lines
4.6 KiB
Diff
145 lines
4.6 KiB
Diff
From 0ef9c539a38510d35daec578dae8491f8a1c651c Mon Sep 17 00:00:00 2001
|
|
Message-ID: <0ef9c539a38510d35daec578dae8491f8a1c651c.1766070439.git.jdenemar@redhat.com>
|
|
From: Jiri Denemark <jdenemar@redhat.com>
|
|
Date: Fri, 5 Dec 2025 16:47:14 +0100
|
|
Subject: [PATCH] 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>
|
|
(cherry picked from commit b6addd42bece693debbf2e95551a2b4d2e1b453f)
|
|
|
|
https://issues.redhat.com/browse/RHEL-102925
|
|
|
|
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
|
|
---
|
|
src/util/virfile.c | 71 ++++++++++++++++++++++++++++++++++++++++++++--
|
|
1 file changed, 69 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/src/util/virfile.c b/src/util/virfile.c
|
|
index a5c9fbe0d9..95fc8ff0e6 100644
|
|
--- 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);
|
|
--
|
|
2.52.0
|