util-lib: use trailing slash in chase_symlinks, fd_is_mount_point, path_is_mount_point

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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2017-10-31 11:08:30 +01:00
parent ca4d708dc4
commit b12d25a8d6
7 changed files with 115 additions and 18 deletions

View File

@ -661,10 +661,19 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
todo += m;
/* Just a single slash? Then we reached the end. */
if (isempty(first) || path_equal(first, "/"))
/* Empty? Then we reached the end. */
if (isempty(first))
break;
/* Just a single slash? Then we reached the end. */
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, "/."))
continue;

View File

@ -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) {

View File

@ -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;

View File

@ -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_;

View File

@ -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);
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);
result = mfree(result);
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);

View File

@ -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);

View File

@ -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();