Skip to content

Commit

Permalink
path-util: Fix path_is_mount_point for files
Browse files Browse the repository at this point in the history 
Commits 27cc6f1 and f25afeb broke path_is_mount_point() for files (such as
/etc/machine-id → /run/machine-id bind mounts) as with the factorization of
fd_is_mount_point() we lost the parent directory. We cannot determine that from
an fd only as openat(fd, "..") only works for directory fds.

Change fd_is_mount_point() to behave like openat(): It now takes a file
descriptor of the containing directory, a file name in it, and flags (which can
be 0 or AT_SYMLINK_FOLLOW). Unlike name_to_handle_at() or openat(), fstatat()
only accepts the inverse flag AT_SYMLINK_NOFOLLOW and complains with EINVAL
about AT_SYMLINK_FOLLOW; so we need to transform the flags for that fallback.

Adjust rm_rf_children() accordingly (only other caller of fd_is_mount_point()
aside from path_is_mount_point()).

Add test cases for files, links, and file bind mounts (the latter will only
work when running as root). Split out a new test_path_is_mount_point() test
case function as it got significantly larger now.
  • Loading branch information
@martinpitt
martinpitt committed May 29, 2015
1 parent 2fe9e87 commit 5d40903
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 24 deletions.
31 changes: 22 additions & 9 deletions src/shared/path-util.c
View file
Expand Up @@ -509,14 +509,15 @@ static int fd_fdinfo_mnt_id(int fd, const char *filename, int flags, int *mnt_id
return safe_atoi(p, mnt_id);
}

int fd_is_mount_point(int fd) {
int fd_is_mount_point(int fd, const char *filename, int flags) {
union file_handle_union h = FILE_HANDLE_INIT, h_parent = FILE_HANDLE_INIT;
int mount_id = -1, mount_id_parent = -1;
bool nosupp = false, check_st_dev = true;
struct stat a, b;
int r;

assert(fd >= 0);
assert(filename);

/* First we will try the name_to_handle_at() syscall, which
* tells us the mount id and an opaque file "handle". It is
Expand All @@ -541,7 +542,7 @@ int fd_is_mount_point(int fd) {
* subvolumes have different st_dev, even though they aren't
* real mounts of their own. */

r = name_to_handle_at(fd, "", &h.handle, &mount_id, AT_EMPTY_PATH);
r = name_to_handle_at(fd, filename, &h.handle, &mount_id, flags);
if (r < 0) {
if (errno == ENOSYS)
/* This kernel does not support name_to_handle_at()
Expand All @@ -558,7 +559,7 @@ int fd_is_mount_point(int fd) {
return -errno;
}

r = name_to_handle_at(fd, "..", &h_parent.handle, &mount_id_parent, 0);
r = name_to_handle_at(fd, "", &h_parent.handle, &mount_id_parent, AT_EMPTY_PATH);
if (r < 0) {
if (errno == EOPNOTSUPP) {
if (nosupp)
Expand Down Expand Up @@ -593,13 +594,13 @@ int fd_is_mount_point(int fd) {
return mount_id != mount_id_parent;

fallback_fdinfo:
r = fd_fdinfo_mnt_id(fd, "", AT_EMPTY_PATH, &mount_id);
r = fd_fdinfo_mnt_id(fd, filename, flags, &mount_id);
if (r == -EOPNOTSUPP)
goto fallback_fstat;
if (r < 0)
return r;

r = fd_fdinfo_mnt_id(fd, "..", 0, &mount_id_parent);
r = fd_fdinfo_mnt_id(fd, "", AT_EMPTY_PATH, &mount_id_parent);
if (r < 0)
return r;

Expand All @@ -615,10 +616,16 @@ int fd_is_mount_point(int fd) {
check_st_dev = false;

fallback_fstat:
if (fstatat(fd, "", &a, AT_EMPTY_PATH) < 0)
/* yay for fstatat() taking a different set of flags than the other
* _at() above */
if (flags & AT_SYMLINK_FOLLOW)
flags &= ~AT_SYMLINK_FOLLOW;
else
flags |= AT_SYMLINK_NOFOLLOW;
if (fstatat(fd, filename, &a, flags) < 0)
return -errno;

if (fstatat(fd, "..", &b, 0) < 0)
if (fstatat(fd, "", &b, AT_EMPTY_PATH) < 0)
return -errno;

/* A directory with same device and inode as its parent? Must
Expand All @@ -632,17 +639,23 @@ int fd_is_mount_point(int fd) {

int path_is_mount_point(const char *t, bool allow_symlink) {
_cleanup_close_ int fd = -1;
_cleanup_free_ char *parent = NULL;
int r;

assert(t);

if (path_equal(t, "/"))
return 1;

fd = openat(AT_FDCWD, t, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|(allow_symlink ? 0 : O_PATH));
r = path_get_parent(t, &parent);

This comment has been minimized.

Sign in to view

Copy link
@haraldh

haraldh Jun 3, 2015

Member

@martinpitt : this broke

condition = condition_new(CONDITION_PATH_IS_MOUNT_POINT, "/bin", false, false);
assert_se(!condition_test(condition));

with /bin being a symlink to usr/bin and /usr being a mount point

path_get_parent("/bin") -> "/"
and then "/" is compared to "/usr/bin"

with AT_FOLLOW_SYMLINK we should probably
path_get_parent(readlink("/bin"))
if (r < 0)
return r;

fd = openat(AT_FDCWD, parent, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_PATH);
if (fd < 0)
return -errno;

return fd_is_mount_point(fd);
return fd_is_mount_point(fd, basename(t), (allow_symlink ? AT_SYMLINK_FOLLOW : 0));
}

int path_is_read_only_fs(const char *path) {
Expand Down
2 changes: 1 addition & 1 deletion src/shared/path-util.h
View file
Expand Up @@ -53,7 +53,7 @@ char** path_strv_make_absolute_cwd(char **l);
char** path_strv_resolve(char **l, const char *prefix);
char** path_strv_resolve_uniq(char **l, const char *prefix);

int fd_is_mount_point(int fd);
int fd_is_mount_point(int fd, const char *filename, int flags);
int path_is_mount_point(const char *path, bool allow_symlink);
int path_is_read_only_fs(const char *path);
int path_is_os_tree(const char *path);
Expand Down
2 changes: 1 addition & 1 deletion src/shared/rm-rf.c
View file
Expand Up @@ -103,7 +103,7 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev) {
}

/* Stop at mount points */
r = fd_is_mount_point(subdir_fd);
r = fd_is_mount_point(fd, de->d_name, 0);
if (r < 0) {
if (ret == 0 && r != -ENOENT)
ret = r;
Expand Down
76 changes: 63 additions & 13 deletions src/test/test-path-util.c
View file
Expand Up @@ -21,6 +21,7 @@

#include <stdio.h>
#include <unistd.h>
#include <sys/mount.h>

#include "path-util.h"
#include "util.h"
Expand Down Expand Up @@ -88,21 +89,9 @@ static void test_path(void) {
test_parent("/aa///file...", "/aa///");
test_parent("file.../", NULL);

assert_se(path_is_mount_point("/", true) > 0);
assert_se(path_is_mount_point("/", false) > 0);

fd = open("/", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY);
assert_se(fd >= 0);
assert_se(fd_is_mount_point(fd) > 0);

assert_se(path_is_mount_point("/proc", true) > 0);
assert_se(path_is_mount_point("/proc", false) > 0);

assert_se(path_is_mount_point("/proc/1", true) == 0);
assert_se(path_is_mount_point("/proc/1", false) == 0);

assert_se(path_is_mount_point("/sys", true) > 0);
assert_se(path_is_mount_point("/sys", false) > 0);
assert_se(fd_is_mount_point(fd, "/", 0) > 0);

{
char p1[] = "aaa/bbb////ccc";
Expand Down Expand Up @@ -322,6 +311,66 @@ static void test_prefix_root(void) {
test_prefix_root_one("/foo///", "//bar", "/foo/bar");
}

static void test_path_is_mount_point(void) {
int fd, rt, rf, rlt, rlf;
char tmp_dir[] = "/tmp/test-path-is-mount-point-XXXXXX";
_cleanup_free_ char *file1 = NULL, *file2 = NULL, *link1 = NULL, *link2 = NULL;

assert_se(path_is_mount_point("/", true) > 0);
assert_se(path_is_mount_point("/", false) > 0);

assert_se(path_is_mount_point("/proc", true) > 0);
assert_se(path_is_mount_point("/proc", false) > 0);

assert_se(path_is_mount_point("/proc/1", true) == 0);
assert_se(path_is_mount_point("/proc/1", false) == 0);

assert_se(path_is_mount_point("/sys", true) > 0);
assert_se(path_is_mount_point("/sys", false) > 0);

/* file mountpoints */
assert_se(mkdtemp(tmp_dir) != NULL);
file1 = path_join(NULL, tmp_dir, "file1");
assert_se(file1);
file2 = path_join(NULL, tmp_dir, "file2");
assert_se(file2);
fd = open(file1, O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0664);
assert_se(fd > 0);
close(fd);
fd = open(file2, O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0664);
assert_se(fd > 0);
close(fd);
link1 = path_join(NULL, tmp_dir, "link1");
assert_se(link1);
assert_se(symlink("file1", link1) == 0);
link2 = path_join(NULL, tmp_dir, "link2");
assert_se(link1);
assert_se(symlink("file2", link2) == 0);

assert_se(path_is_mount_point(file1, true) == 0);
assert_se(path_is_mount_point(file1, false) == 0);
assert_se(path_is_mount_point(link1, true) == 0);
assert_se(path_is_mount_point(link1, false) == 0);

/* this test will only work as root */
if (mount(file1, file2, NULL, MS_BIND, NULL) >= 0) {
rf = path_is_mount_point(file2, false);
rt = path_is_mount_point(file2, true);
rlf = path_is_mount_point(link2, false);
rlt = path_is_mount_point(link2, true);

assert_se(umount(file2) == 0);

assert_se(rf == 1);
assert_se(rt == 1);
assert_se(rlf == 0);
assert_se(rlt == 1);
} else
printf("Skipping bind mount file test: %m\n");

assert_se(rm_rf(tmp_dir, REMOVE_ROOT|REMOVE_PHYSICAL) == 0);
}

int main(int argc, char **argv) {
test_path();
test_find_binary(argv[0], true);
Expand All @@ -333,6 +382,7 @@ int main(int argc, char **argv) {
test_strv_resolve();
test_path_startswith();
test_prefix_root();
test_path_is_mount_point();

return 0;
}

0 comments on commit 5d40903

Please sign in to comment.