[systemd-devel] [PATCH] path-util: Fix path_is_mount_point for files
Lennart Poettering
lennart at poettering.net
Thu May 28 10:44:43 PDT 2015
On Wed, 27.05.15 10:07, Martin Pitt (martin.pitt at ubuntu.com) wrote:
> -int fd_is_mount_point(int fd) {
> +int fd_is_mount_point(int fd, const char *parent) {
Hmm, now I am confused? Why "parent"?
I really think this should work as close as the usual *at() calls
work. i.e. take a dir fd as first argument, and a filename
*within*that*directory* to check. Maybe even give it the _at() suffix:
int fd_is_mount_point_at(int fd, const char *filename, int flags);
int path_is_mount_point(const char *path, int flags);
path_is_mount_point() simply seperates the last part of the path,
opens its parent directory, and then invokes fd_is_mount_point_at()
with the parent dir and the last component...
Or am I missing something?
> 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;
> @@ -558,7 +558,11 @@ int fd_is_mount_point(int fd) {
> return -errno;
> }
>
> - r = name_to_handle_at(fd, "..", &h_parent.handle, &mount_id_parent, 0);
> + if (parent)
> + r = name_to_handle_at(AT_FDCWD, parent, &h_parent.handle, &mount_id_parent, 0);
> + else
> + /* this only works for directories */
> + r = name_to_handle_at(fd, "..", &h_parent.handle, &mount_id_parent, 0);
> if (r < 0) {
> if (errno == EOPNOTSUPP) {
> if (nosupp)
> @@ -599,7 +603,11 @@ fallback_fdinfo:
> if (r < 0)
> return r;
>
> - r = fd_fdinfo_mnt_id(fd, "..", 0, &mount_id_parent);
> + if (parent)
> + r = fd_fdinfo_mnt_id(AT_FDCWD, parent, 0, &mount_id_parent);
> + else
> + /* this only works for directories */
> + r = fd_fdinfo_mnt_id(fd, "..", 0, &mount_id_parent);
> if (r < 0)
> return r;
>
> @@ -618,7 +626,11 @@ fallback_fstat:
> if (fstatat(fd, "", &a, AT_EMPTY_PATH) < 0)
> return -errno;
>
> - if (fstatat(fd, "..", &b, 0) < 0)
> + if (parent)
> + r = fstatat(AT_FDCWD, parent, &b, 0);
> + else
> + r = fstatat(fd, "..", &b, 0);
> + if (r < 0)
> return -errno;
>
> /* A directory with same device and inode as its parent? Must
> @@ -632,17 +644,23 @@ fallback_fstat:
>
> 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);
> + if (r < 0)
> + return r;
> +
> + fd = openat(AT_FDCWD, t, O_RDONLY|O_NONBLOCK|O_CLOEXEC|(allow_symlink ? 0 : O_PATH|O_NOFOLLOW));
> if (fd < 0)
> return -errno;
>
> - return fd_is_mount_point(fd);
> + return fd_is_mount_point(fd, parent);
> }
>
> int path_is_read_only_fs(const char *path) {
> diff --git a/src/shared/path-util.h b/src/shared/path-util.h
> index 4f45cfd..cf5143f 100644
> --- a/src/shared/path-util.h
> +++ b/src/shared/path-util.h
> @@ -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 *parent);
> 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);
> diff --git a/src/shared/rm-rf.c b/src/shared/rm-rf.c
> index a89e8af..e84bb10 100644
> --- a/src/shared/rm-rf.c
> +++ b/src/shared/rm-rf.c
> @@ -102,8 +102,8 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev) {
> continue;
> }
>
> - /* Stop at mount points */
> - r = fd_is_mount_point(subdir_fd);
> + /* Stop at directory mount points */
> + r = fd_is_mount_point(subdir_fd, NULL);
> if (r < 0) {
> if (ret == 0 && r != -ENOENT)
> ret = r;
> diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
> index 09f0f2f..0a03941 100644
> --- a/src/test/test-path-util.c
> +++ b/src/test/test-path-util.c
> @@ -21,6 +21,7 @@
>
> #include <stdio.h>
> #include <unistd.h>
> +#include <sys/mount.h>
>
> #include "path-util.h"
> #include "util.h"
> @@ -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, NULL) > 0);
>
> {
> char p1[] = "aaa/bbb////ccc";
> @@ -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);
> @@ -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;
> }
> --
> 2.1.4
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
More information about the systemd-devel
mailing list