From: Jiri Denemark <jdenemar@redhat.com> 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> --- 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 f195d02e29..b0f4041df4 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