From b12d25a8d631af00b200e7aa9dbba6ba4a4a59ff Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 Oct 2017 11:08:30 +0100 Subject: [PATCH] util-lib: use trailing slash in chase_symlinks, fd_is_mount_point, path_is_mount_point MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The kernel will reply with -ENOTDIR when we try to access a non-directory under a name which ends with a slash. But our functions would strip the trailing slash under various circumstances. Keep the trailing slash, so that path_is_mount_point("/path/to/file/") return -ENOTDIR when /path/to/file/ is a file. Tests are added for this change in behaviour. Also, when called with a trailing slash, path_is_mount_point() would get "" from basename(), and call name_to_handle_at(3, "", ...), and always return -ENOENT. Now it'll return -ENOTDIR if the mount point is a file, and true if it is a directory and a mount point. v2: - use strip_trailing_chars() v3: - instead of stripping trailing chars(), do the opposite — preserve them. --- src/basic/fs-util.c | 11 ++++++++- src/basic/mount-util.c | 3 ++- src/basic/path-util.c | 28 +++++++++++++++++++++++ src/basic/path-util.h | 1 + src/test/test-fs-util.c | 47 +++++++++++++++++++++++++++----------- src/test/test-mount-util.c | 24 ++++++++++++++++++- src/test/test-path-util.c | 15 ++++++++++++ 7 files changed, 113 insertions(+), 16 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index b5fd95ae8e0..e74e75a41ca 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -661,9 +661,18 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, todo += m; + /* Empty? Then we reached the end. */ + if (isempty(first)) + break; + /* Just a single slash? Then we reached the end. */ - if (isempty(first) || path_equal(first, "/")) + if (path_equal(first, "/")) { + /* Preserve the trailing slash */ + if (!strextend(&done, "/", NULL)) + return -ENOMEM; + break; + } /* Just a dot? Then let's eat this up. */ if (path_equal(first, "/.")) diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c index e32502308a7..51c84491be9 100644 --- a/src/basic/mount-util.c +++ b/src/basic/mount-util.c @@ -277,6 +277,7 @@ int path_is_mount_point(const char *t, const char *root, int flags) { int r; assert(t); + assert((flags & ~AT_SYMLINK_FOLLOW) == 0); if (path_equal(t, "/")) return 1; @@ -301,7 +302,7 @@ int path_is_mount_point(const char *t, const char *root, int flags) { if (fd < 0) return -errno; - return fd_is_mount_point(fd, basename(t), flags); + return fd_is_mount_point(fd, last_path_component(t), flags); } int path_get_mnt_id(const char *path, int *ret) { diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 883856abc22..059e4fcf6a5 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -703,6 +703,34 @@ char* dirname_malloc(const char *path) { return dir2; } +const char *last_path_component(const char *path) { + /* Finds the last component of the path, preserving the + * optional trailing slash that signifies a directory. + * a/b/c → c + * a/b/c/ → c/ + * / → / + * // → / + * /foo/a → a + * /foo/a/ → a/ + * This is different than basename, which returns "" when + * a trailing slash is present. + */ + + unsigned l, k; + + l = k = strlen(path); + while (k > 0 && path[k-1] == '/') + k--; + + if (k == 0) /* the root directory */ + return path + l - 1; + + while (k > 0 && path[k-1] != '/') + k--; + + return path + k; +} + bool filename_is_valid(const char *p) { const char *e; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 9bd783eaf45..f79cdf928ec 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -130,6 +130,7 @@ char *prefix_root(const char *root, const char *path); int parse_path_argument_and_warn(const char *path, bool suppress_root, char **arg); char* dirname_malloc(const char *path); +const char *last_path_component(const char *path); bool filename_is_valid(const char *p) _pure_; bool path_is_normalized(const char *p) _pure_; diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 3c958917bb6..83ddc398b88 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -35,7 +35,7 @@ static void test_chase_symlinks(void) { _cleanup_free_ char *result = NULL; char temp[] = "/tmp/test-chase.XXXXXX"; - const char *top, *p, *q; + const char *top, *p, *pslash, *q, *qslash; int r; assert_se(mkdtemp(temp)); @@ -66,93 +66,114 @@ static void test_chase_symlinks(void) { r = chase_symlinks(p, NULL, 0, &result); assert_se(r > 0); assert_se(path_equal(result, "/usr")); + result = mfree(result); + pslash = strjoina(p, "/"); + r = chase_symlinks(pslash, NULL, 0, &result); + assert_se(r > 0); + assert_se(path_equal(result, "/usr/")); result = mfree(result); + r = chase_symlinks(p, temp, 0, &result); assert_se(r == -ENOENT); + r = chase_symlinks(pslash, temp, 0, &result); + assert_se(r == -ENOENT); + q = strjoina(temp, "/usr"); r = chase_symlinks(p, temp, CHASE_NONEXISTENT, &result); assert_se(r == 0); assert_se(path_equal(result, q)); + result = mfree(result); - assert_se(mkdir(q, 0700) >= 0); + qslash = strjoina(q, "/"); + r = chase_symlinks(pslash, temp, CHASE_NONEXISTENT, &result); + assert_se(r == 0); + assert_se(path_equal(result, qslash)); result = mfree(result); + + assert_se(mkdir(q, 0700) >= 0); + r = chase_symlinks(p, temp, 0, &result); assert_se(r > 0); assert_se(path_equal(result, q)); + result = mfree(result); + + r = chase_symlinks(pslash, temp, 0, &result); + assert_se(r > 0); + assert_se(path_equal(result, qslash)); + result = mfree(result); p = strjoina(temp, "/slash"); assert_se(symlink("/", p) >= 0); - result = mfree(result); r = chase_symlinks(p, NULL, 0, &result); assert_se(r > 0); assert_se(path_equal(result, "/")); - result = mfree(result); + r = chase_symlinks(p, temp, 0, &result); assert_se(r > 0); assert_se(path_equal(result, temp)); + result = mfree(result); /* Paths that would "escape" outside of the "root" */ p = strjoina(temp, "/6dots"); assert_se(symlink("../../..", p) >= 0); - result = mfree(result); r = chase_symlinks(p, temp, 0, &result); assert_se(r > 0 && path_equal(result, temp)); + result = mfree(result); p = strjoina(temp, "/6dotsusr"); assert_se(symlink("../../../usr", p) >= 0); - result = mfree(result); r = chase_symlinks(p, temp, 0, &result); assert_se(r > 0 && path_equal(result, q)); + result = mfree(result); p = strjoina(temp, "/top/8dotsusr"); assert_se(symlink("../../../../usr", p) >= 0); - result = mfree(result); r = chase_symlinks(p, temp, 0, &result); assert_se(r > 0 && path_equal(result, q)); + result = mfree(result); /* Paths that contain repeated slashes */ p = strjoina(temp, "/slashslash"); assert_se(symlink("///usr///", p) >= 0); - result = mfree(result); r = chase_symlinks(p, NULL, 0, &result); assert_se(r > 0); assert_se(path_equal(result, "/usr")); - result = mfree(result); + r = chase_symlinks(p, temp, 0, &result); assert_se(r > 0); assert_se(path_equal(result, q)); + result = mfree(result); /* Paths using . */ - result = mfree(result); r = chase_symlinks("/etc/./.././", NULL, 0, &result); assert_se(r > 0); assert_se(path_equal(result, "/")); - result = mfree(result); + r = chase_symlinks("/etc/./.././", "/etc", 0, &result); assert_se(r > 0 && path_equal(result, "/etc")); - result = mfree(result); + r = chase_symlinks("/etc/machine-id/foo", NULL, 0, &result); assert_se(r == -ENOTDIR); + result = mfree(result); /* Path that loops back to self */ - result = mfree(result); p = strjoina(temp, "/recursive-symlink"); assert_se(symlink("recursive-symlink", p) >= 0); r = chase_symlinks(p, NULL, 0, &result); diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c index b3434bd12cb..09a624842c8 100644 --- a/src/test/test-mount-util.c +++ b/src/test/test-mount-util.c @@ -111,15 +111,23 @@ static void test_path_is_mount_point(void) { assert_se(path_is_mount_point("/", NULL, AT_SYMLINK_FOLLOW) > 0); assert_se(path_is_mount_point("/", NULL, 0) > 0); + assert_se(path_is_mount_point("//", NULL, AT_SYMLINK_FOLLOW) > 0); + assert_se(path_is_mount_point("//", NULL, 0) > 0); assert_se(path_is_mount_point("/proc", NULL, AT_SYMLINK_FOLLOW) > 0); assert_se(path_is_mount_point("/proc", NULL, 0) > 0); + assert_se(path_is_mount_point("/proc/", NULL, AT_SYMLINK_FOLLOW) > 0); + assert_se(path_is_mount_point("/proc/", NULL, 0) > 0); assert_se(path_is_mount_point("/proc/1", NULL, AT_SYMLINK_FOLLOW) == 0); assert_se(path_is_mount_point("/proc/1", NULL, 0) == 0); + assert_se(path_is_mount_point("/proc/1/", NULL, AT_SYMLINK_FOLLOW) == 0); + assert_se(path_is_mount_point("/proc/1/", NULL, 0) == 0); assert_se(path_is_mount_point("/sys", NULL, AT_SYMLINK_FOLLOW) > 0); assert_se(path_is_mount_point("/sys", NULL, 0) > 0); + assert_se(path_is_mount_point("/sys/", NULL, AT_SYMLINK_FOLLOW) > 0); + assert_se(path_is_mount_point("/sys/", NULL, 0) > 0); /* we'll create a hierarchy of different kinds of dir/file/link * layouts: @@ -191,12 +199,21 @@ static void test_path_is_mount_point(void) { /* these tests will only work as root */ if (mount(file1, file2, NULL, MS_BIND, NULL) >= 0) { - int rt, rf, rlt, rlf, rl1t, rl1f; + int rf, rt, rdf, rdt, rlf, rlt, rl1f, rl1t; + const char *file2d; /* files */ /* capture results in vars, to avoid dangling mounts on failure */ + log_info("%s: %s", __func__, file2); rf = path_is_mount_point(file2, NULL, 0); rt = path_is_mount_point(file2, NULL, AT_SYMLINK_FOLLOW); + + file2d = strjoina(file2, "/"); + log_info("%s: %s", __func__, file2d); + rdf = path_is_mount_point(file2d, NULL, 0); + rdt = path_is_mount_point(file2d, NULL, AT_SYMLINK_FOLLOW); + + log_info("%s: %s", __func__, link2); rlf = path_is_mount_point(link2, NULL, 0); rlt = path_is_mount_point(link2, NULL, AT_SYMLINK_FOLLOW); @@ -204,6 +221,8 @@ static void test_path_is_mount_point(void) { assert_se(rf == 1); assert_se(rt == 1); + assert_se(rdf == -ENOTDIR); + assert_se(rdt == -ENOTDIR); assert_se(rlf == 0); assert_se(rlt == 1); @@ -216,10 +235,13 @@ static void test_path_is_mount_point(void) { assert_se(mount(dir2, dir1, NULL, MS_BIND, NULL) >= 0); + log_info("%s: %s", __func__, dir1); rf = path_is_mount_point(dir1, NULL, 0); rt = path_is_mount_point(dir1, NULL, AT_SYMLINK_FOLLOW); + log_info("%s: %s", __func__, dirlink1); rlf = path_is_mount_point(dirlink1, NULL, 0); rlt = path_is_mount_point(dirlink1, NULL, AT_SYMLINK_FOLLOW); + log_info("%s: %s", __func__, dirlink1file); /* its parent is a mount point, but not /file itself */ rl1f = path_is_mount_point(dirlink1file, NULL, 0); rl1t = path_is_mount_point(dirlink1file, NULL, AT_SYMLINK_FOLLOW); diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index fed829f1f72..21d52f5d6e6 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -399,6 +399,20 @@ static void test_file_in_same_dir(void) { free(t); } +static void test_last_path_component(void) { + assert_se(streq(last_path_component("a/b/c"), "c")); + assert_se(streq(last_path_component("a/b/c/"), "c/")); + assert_se(streq(last_path_component("/"), "/")); + assert_se(streq(last_path_component("//"), "/")); + assert_se(streq(last_path_component("///"), "/")); + assert_se(streq(last_path_component("."), ".")); + assert_se(streq(last_path_component("./."), ".")); + assert_se(streq(last_path_component("././"), "./")); + assert_se(streq(last_path_component("././/"), ".//")); + assert_se(streq(last_path_component("/foo/a"), "a")); + assert_se(streq(last_path_component("/foo/a/"), "a/")); +} + static void test_filename_is_valid(void) { char foo[FILENAME_MAX+2]; int i; @@ -484,6 +498,7 @@ int main(int argc, char **argv) { test_path_startswith(); test_prefix_root(); test_file_in_same_dir(); + test_last_path_component(); test_filename_is_valid(); test_hidden_or_backup_file(); test_skip_dev_prefix(); -- 2.39.2