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 4cc0a01df8ef0a289b8ef034661ed5574fe353f6..86a2b7b25cc7788888b9d0345a1281e57a9ba6dc 100644 (file)
--- 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 668a44733bc93a65b525e34d94336131bd9c152e..3fc5f88d26e1e9299052e3a7c5faca9dd9c096fa 100644 (file)
--- 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;