From: Daan De Meyer Date: Fri, 6 Jan 2023 12:49:09 +0000 (+0100) Subject: chase-symlinks: chase_symlinks_at() AT_FDCWD fixes X-Git-Tag: v254-rc1~1250^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c677e13c35ff2d475199bcc9432892396485b6b8;p=thirdparty%2Fsystemd.git chase-symlinks: chase_symlinks_at() AT_FDCWD fixes - Whether we should return an absolute path or not is irrelevant to whether CHASE_AT_RESOLVE_IN_ROOT is set. We should only return an absolute path if we are provided one and the directory file descriptor is AT_FDCWD - When the directory file descriptor is AT_FDCWD, we should always resolve symlinks against "/". Currently, if the directory file descriptor is AT_FDCWD and CHASE_AT_RESOLVE_IN_ROOT is set, we resolve symlinks against the current working directory which is almost always not going to be what the caller wants. - Currently, if we provide an absolute path with a positive directory file descriptor without CHASE_AT_RESOLVE_IN_ROOT SET, we interpret the path relative to "/" instead of the given directory file descriptor. Let's make sure that when we're given a positive directory file descriptor, we always resolve the given path relative to it. --- diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 4cc0a01df8e..86a2b7b25cc 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -102,20 +102,22 @@ int chase_symlinks_at( path = "."; /* This function resolves symlinks of the path relative to the given directory file descriptor. If - * CHASE_SYMLINKS_RESOLVE_IN_ROOT is specified, symlinks are resolved relative to the given directory - * file descriptor. Otherwise, they are resolved relative to the root directory of the host. + * CHASE_SYMLINKS_RESOLVE_IN_ROOT is specified and a directory file descriptor is provided, symlinks + * are resolved relative to the given directory file descriptor. Otherwise, they are resolved + * relative to the root directory of the host. * - * Note that when CHASE_SYMLINKS_RESOLVE_IN_ROOT is specified and we find an absolute symlink, it is - * resolved relative to given directory file descriptor and not the root of the host. Also, when - * following relative symlinks, this functions ensure they cannot be used to "escape" the given - * directory file descriptor. The "path" parameter is always interpreted relative to the given - * directory file descriptor. If the given directory file descriptor is AT_FDCWD and "path" is - * absolute, it is interpreted relative to the root directory of the host. + * Note that when a positive directory file descriptor is provided and CHASE_AT_RESOLVE_IN_ROOT is + * specified and we find an absolute symlink, it is resolved relative to given directory file + * descriptor and not the root of the host. Also, when following relative symlinks, this functions + * ensures they cannot be used to "escape" the given directory file descriptor. If a positive + * directory file descriptor is provided, the "path" parameter is always interpreted relative to the + * given directory file descriptor, even if it is absolute. If the given directory file descriptor is + * AT_FDCWD and "path" is absolute, it is interpreted relative to the root directory of the host. * * If "dir_fd" is a valid directory fd, "path" is an absolute path and "ret_path" is not NULL, this * functions returns a relative path in "ret_path" because openat() like functions generally ignore * the directory fd if they are provided with an absolute path. On the other hand, if "dir_fd" is - * AT_FDCWD and "path" is an absolute path, we need to return an absolute path in "ret_path" because + * AT_FDCWD and "path" is an absolute path, we return an absolute path in "ret_path" because * otherwise, if the caller passes the returned relative path to another openat() like function, it * would be resolved relative to the current working directory instead of to "/". * @@ -177,21 +179,30 @@ int chase_symlinks_at( if (!buffer) return -ENOMEM; - bool need_absolute = !FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT) && path_is_absolute(path); + /* If we receive an absolute path together with AT_FDCWD, we need to return an absolute path, because + * a relative path would be interpreted relative to the current working directory. */ + bool need_absolute = dir_fd == AT_FDCWD && path_is_absolute(path); if (need_absolute) { done = strdup("/"); if (!done) return -ENOMEM; } - if (FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT)) + /* If we get AT_FDCWD, we always resolve symlinks relative to the host's root. Only if a positive + * directory file descriptor is provided will we look at CHASE_AT_RESOLVE_IN_ROOT to determine + * whether to resolve symlinks in it or not. */ + if (dir_fd >= 0 && FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT)) root_fd = openat(dir_fd, ".", O_CLOEXEC|O_DIRECTORY|O_PATH); else root_fd = open("/", O_CLOEXEC|O_DIRECTORY|O_PATH); if (root_fd < 0) return -errno; - if (FLAGS_SET(flags, CHASE_AT_RESOLVE_IN_ROOT) || !path_is_absolute(path)) + /* If a positive directory file descriptor is provided, always resolve the given path relative to it, + * regardless of whether it is absolute or not. If we get AT_FDCWD, follow regular openat() + * semantics, if the path is relative, resolve against the current working directory. Otherwise, + * resolve against root. */ + if (dir_fd >= 0 || !path_is_absolute(path)) fd = openat(dir_fd, ".", O_CLOEXEC|O_DIRECTORY|O_PATH); else fd = open("/", O_CLOEXEC|O_DIRECTORY|O_PATH); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 668a44733bc..3fc5f88d26e 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -430,6 +430,42 @@ TEST(chase_symlinks) { assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); } +TEST(chase_symlinks_at) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_close_ int tfd = -EBADF, fd = -EBADF; + _cleanup_free_ char *result = NULL; + const char *p; + + assert_se((tfd = mkdtemp_open(NULL, 0, &t)) >= 0); + + /* Test that AT_FDCWD with CHASE_AT_RESOLVE_IN_ROOT resolves against / and not the current working + * directory. */ + + assert_se(symlinkat("/usr", tfd, "abc") >= 0); + + p = strjoina(t, "/abc"); + assert_se(chase_symlinks_at(AT_FDCWD, p, CHASE_AT_RESOLVE_IN_ROOT, &result, NULL) >= 0); + assert_se(streq(result, "/usr")); + result = mfree(result); + + /* Test that absolute path or not are the same when resolving relative to a directory file + * descriptor and that we always get a relative path back. */ + + assert_se(fd = openat(tfd, "def", O_CREAT|O_CLOEXEC, 0700) >= 0); + fd = safe_close(fd); + assert_se(symlinkat("/def", tfd, "qed") >= 0); + assert_se(chase_symlinks_at(tfd, "qed", CHASE_AT_RESOLVE_IN_ROOT, &result, NULL) >= 0); + assert_se(streq(result, "def")); + result = mfree(result); + assert_se(chase_symlinks_at(tfd, "/qed", CHASE_AT_RESOLVE_IN_ROOT, &result, NULL) >= 0); + assert_se(streq(result, "def")); + result = mfree(result); + + /* Valid directory file descriptor without CHASE_AT_RESOLVE_IN_ROOT should resolve symlinks against + * host's root. */ + assert_se(chase_symlinks_at(tfd, "/qed", 0, &result, NULL) == -ENOENT); +} + TEST(unlink_noerrno) { char *name; int fd;