1
0
mirror of https://github.com/systemd/systemd.git synced 2025-01-11 09:18:07 +03:00

chase: fix corner case when using CHASE_PARENT with a path ending in ".."

If we use CHASE_PARENT on a path ending in ".." then things are a bit
weird, because we the last path we look at is actually the *parent* and not
the *child* of the preceeding path. Hence we cannot just return the 2nd
to last fd we look at. We have to correct it, by going *two* levels up,
to get to the actual parent, and make sure CHASE_PARENT does what it
should.

Example: for the path /a/b/c chase() with CHASE_PARENT will return
/a/b/c as path, and the fd returned points to /a/b. All good.  But now,
for the path /a/b/c/.. chase() with CHASE_PARENT would previously return
/a/b as path (which is OK) but the fd would point to /a/b/c, which is
*not* the parent of /a/b, after all! To get to the actual parent of
/a/b we have to go *two* levels up to get to /a.

Very confusing. But that's what we here for, no?

@mrc0mmand ran into this in https://github.com/systemd/systemd/pull/28891#issuecomment-1782833722
This commit is contained in:
Lennart Poettering 2023-11-01 12:46:17 +01:00 committed by Luca Boccassi
parent 7c2fd96dba
commit 9c21cfdd7d
2 changed files with 48 additions and 1 deletions

View File

@ -335,8 +335,28 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int
unsafe_transition(&st, &st_parent))
return log_unsafe_transition(fd, fd_parent, path, flags);
if (FLAGS_SET(flags, CHASE_PARENT) && isempty(todo))
/* If the path ends on a "..", and CHASE_PARENT is specified then our current 'fd' is
* the child of the returned normalized path, not the parent as requested. To correct
* this we have to go *two* levels up. */
if (FLAGS_SET(flags, CHASE_PARENT) && isempty(todo)) {
_cleanup_close_ int fd_grandparent = -EBADF;
struct stat st_grandparent;
fd_grandparent = openat(fd_parent, "..", O_CLOEXEC|O_NOFOLLOW|O_PATH|O_DIRECTORY);
if (fd_grandparent < 0)
return -errno;
if (fstat(fd_grandparent, &st_grandparent) < 0)
return -errno;
if (FLAGS_SET(flags, CHASE_SAFE) &&
unsafe_transition(&st_parent, &st_grandparent))
return log_unsafe_transition(fd_parent, fd_grandparent, path, flags);
st = st_grandparent;
close_and_replace(fd, fd_grandparent);
break;
}
/* update fd and stat */
st = st_parent;

View File

@ -721,6 +721,33 @@ TEST(chaseat_prefix_root) {
assert_se(streq(ret, expected));
}
TEST(trailing_dot_dot) {
_cleanup_free_ char *path = NULL, *fdpath = NULL;
_cleanup_close_ int fd = -EBADF;
assert_se(chase("/usr/..", NULL, CHASE_PARENT, &path, &fd) >= 0);
assert_se(path_equal(path, "/"));
assert_se(fd_get_path(fd, &fdpath) >= 0);
assert_se(path_equal(fdpath, "/"));
path = mfree(path);
fdpath = mfree(fdpath);
fd = safe_close(fd);
_cleanup_(rm_rf_physical_and_freep) char *t = NULL;
assert_se(mkdtemp_malloc(NULL, &t) >= 0);
_cleanup_free_ char *sub = ASSERT_PTR(path_join(t, "a/b/c/d"));
assert_se(mkdir_p(sub, 0700) >= 0);
_cleanup_free_ char *suffixed = ASSERT_PTR(path_join(sub, ".."));
assert_se(chase(suffixed, NULL, CHASE_PARENT, &path, &fd) >= 0);
_cleanup_free_ char *expected1 = ASSERT_PTR(path_join(t, "a/b/c"));
_cleanup_free_ char *expected2 = ASSERT_PTR(path_join(t, "a/b"));
assert_se(path_equal(path, expected1));
assert_se(fd_get_path(fd, &fdpath) >= 0);
assert_se(path_equal(fdpath, expected2));
}
static int intro(void) {
arg_test_dir = saved_argv[1];
return EXIT_SUCCESS;