From f63b66b6347a8d8e5e6930a939d1997bfd8e2e7c Mon Sep 17 00:00:00 2001 From: Jan Synacek Date: Fri, 28 Jul 2017 15:31:50 +0200 Subject: [PATCH] path-util: make use of "mnt_id" field exported in /proc/self/fdinfo/ This commit is not a backport of a specific commit. It includes parts of several upstream commits (3f72b427b44f39a1aec6806dad6f6b57103ae9ed, 5d409034017e9f9f8c4392157d95511fc2e05d87 and others). The main goal was to bring path_is_mount_point() up to date, which meant introducing fd_fdinfo_mnt_id() and fd_is_mount_point(). These were needed mainly because we need to determine mount points based on /proc/self/fdinfo/ in containers. Also, there are more places in the code where checks for mount points are performed, which would benefit from this fix as well. Additionally, corresponding tests has been added. Resolves: #1472439 --- src/core/automount.c | 2 +- src/core/machine-id-setup.c | 2 +- src/core/mount-setup.c | 2 +- src/efi-boot-generator/efi-boot-generator.c | 2 +- src/gpt-auto-generator/gpt-auto-generator.c | 2 +- src/login/logind-user.c | 2 +- src/nspawn/nspawn.c | 10 +- src/shared/cgroup-util.c | 2 +- src/shared/condition.c | 2 +- src/shared/path-util.c | 209 +++++++++++++++----- src/shared/path-util.h | 3 +- src/test/test-path-util.c | 66 ++++++- 12 files changed, 242 insertions(+), 62 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 4e066613d..eedd9b824 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -749,7 +749,7 @@ static int automount_start(Unit *u) { assert(a); assert(a->state == AUTOMOUNT_DEAD || a->state == AUTOMOUNT_FAILED); - if (path_is_mount_point(a->where, false)) { + if (path_is_mount_point(a->where, 0)) { log_unit_error(u->id, "Path %s is already a mount point, refusing start for %s", a->where, u->id); diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c index d00a53246..1121d373f 100644 --- a/src/core/machine-id-setup.c +++ b/src/core/machine-id-setup.c @@ -203,7 +203,7 @@ int machine_id_commit(const char *root) { etc_machine_id = path_kill_slashes(x); } - r = path_is_mount_point(etc_machine_id, false); + r = path_is_mount_point(etc_machine_id, 0); if (r < 0) return log_error_errno(r, "Failed to determine whether %s is a mount point: %m", etc_machine_id); if (r == 0) { diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index 521545e5c..2b8fbab1a 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -160,7 +160,7 @@ static int mount_one(const MountPoint *p, bool relabel) { if (relabel) label_fix(p->where, true, true); - r = path_is_mount_point(p->where, true); + r = path_is_mount_point(p->where, AT_SYMLINK_FOLLOW); if (r < 0) return r; diff --git a/src/efi-boot-generator/efi-boot-generator.c b/src/efi-boot-generator/efi-boot-generator.c index b3ff3a8b7..5492b1994 100644 --- a/src/efi-boot-generator/efi-boot-generator.c +++ b/src/efi-boot-generator/efi-boot-generator.c @@ -69,7 +69,7 @@ int main(int argc, char *argv[]) { return EXIT_SUCCESS; } - if (path_is_mount_point("/boot", true) <= 0 && + if (path_is_mount_point("/boot", AT_SYMLINK_FOLLOW) <= 0 && dir_is_empty("/boot") <= 0) { log_debug("/boot already populated, exiting."); return EXIT_SUCCESS; diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 00a2141a5..d7b047118 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -299,7 +299,7 @@ static int probe_and_add_mount( assert(where); assert(description); - if (path_is_mount_point(where, true) <= 0 && + if (path_is_mount_point(where, AT_SYMLINK_FOLLOW) <= 0 && dir_is_empty(where) <= 0) { log_debug("%s already populated, ignoring.", where); return 0; diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 4298704ce..912c50ebd 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -320,7 +320,7 @@ static int user_mkdir_runtime_path(User *u) { } else p = u->runtime_path; - if (path_is_mount_point(p, false) <= 0) { + if (path_is_mount_point(p, 0) <= 0) { _cleanup_free_ char *t = NULL; (void) mkdir(p, 0700); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index ea365b3f9..a90a3a5d7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -863,7 +863,7 @@ static int mount_all(const char *dest) { if (!where) return log_oom(); - t = path_is_mount_point(where, true); + t = path_is_mount_point(where, AT_SYMLINK_FOLLOW); if (t < 0) { log_error_errno(t, "Failed to detect whether %s is a mount point: %m", where); @@ -989,7 +989,7 @@ static int mount_cgroup_hierarchy(const char *dest, const char *controller, cons to = strjoina(dest, "/sys/fs/cgroup/", hierarchy); - r = path_is_mount_point(to, false); + r = path_is_mount_point(to, 0); if (r < 0) return log_error_errno(r, "Failed to determine if %s is mounted already: %m", to); if (r > 0) @@ -1787,7 +1787,7 @@ static int setup_journal(const char *directory) { if (!p || !q) return log_oom(); - if (path_is_mount_point(p, false) > 0) { + if (path_is_mount_point(p, 0) > 0) { if (arg_link_journal != LINK_AUTO) { log_error("%s: already a mount point, refusing to use for journal", p); return -EEXIST; @@ -1796,7 +1796,7 @@ static int setup_journal(const char *directory) { return 0; } - if (path_is_mount_point(q, false) > 0) { + if (path_is_mount_point(q, 0) > 0) { if (arg_link_journal != LINK_AUTO) { log_error("%s: already a mount point, refusing to use for journal", q); return -EEXIST; @@ -3665,7 +3665,7 @@ int main(int argc, char *argv[]) { * the specified is not a mount point we * create the new snapshot in the parent * directory, just next to it. */ - r = path_is_mount_point(arg_directory, false); + r = path_is_mount_point(arg_directory, 0); if (r < 0) { log_error_errno(r, "Failed to determine whether directory %s is mount point: %m", arg_directory); goto finish; diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c index c5d9e4bb5..cf085cb5f 100644 --- a/src/shared/cgroup-util.c +++ b/src/shared/cgroup-util.c @@ -488,7 +488,7 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch if (_unlikely_(!good)) { int r; - r = path_is_mount_point("/sys/fs/cgroup", false); + r = path_is_mount_point("/sys/fs/cgroup", 0); if (r <= 0) return r < 0 ? r : -ENOENT; diff --git a/src/shared/condition.c b/src/shared/condition.c index 796cc520d..0d2cd2bc3 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -350,7 +350,7 @@ static int condition_test_path_is_mount_point(Condition *c) { assert(c->parameter); assert(c->type == CONDITION_PATH_IS_MOUNT_POINT); - return path_is_mount_point(c->parameter, true) > 0; + return path_is_mount_point(c->parameter, AT_SYMLINK_FOLLOW) > 0; } static int condition_test_path_is_read_write(Condition *c) { diff --git a/src/shared/path-util.c b/src/shared/path-util.c index 1181ffb9d..0f252ec26 100644 --- a/src/shared/path-util.c +++ b/src/shared/path-util.c @@ -36,6 +36,7 @@ #include "strv.h" #include "path-util.h" #include "missing.h" +#include "fileio.h" bool path_is_absolute(const char *p) { return p[0] == '/'; @@ -473,87 +474,203 @@ char* path_join(const char *root, const char *path, const char *rest) { NULL); } -int path_is_mount_point(const char *t, bool allow_symlink) { +static int fd_fdinfo_mnt_id(int fd, const char *filename, int flags, int *mnt_id) { + char path[strlen("/proc/self/fdinfo/") + DECIMAL_STR_MAX(int)]; + _cleanup_free_ char *fdinfo = NULL; + _cleanup_close_ int subfd = -1; + char *p; + int r; + + if ((flags & AT_EMPTY_PATH) && isempty(filename)) + xsprintf(path, "/proc/self/fdinfo/%i", fd); + else { + subfd = openat(fd, filename, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_PATH); + if (subfd < 0) + return -errno; + + xsprintf(path, "/proc/self/fdinfo/%i", subfd); + } + + r = read_full_file(path, &fdinfo, NULL); + if (r == -ENOENT) /* The fdinfo directory is a relatively new addition */ + return -EOPNOTSUPP; + if (r < 0) + return -errno; + + p = startswith(fdinfo, "mnt_id:"); + if (!p) { + p = strstr(fdinfo, "\nmnt_id:"); + if (!p) /* The mnt_id field is a relatively new addition */ + return -EOPNOTSUPP; + + p += 8; + } - union file_handle_union h = FILE_HANDLE_INIT; + p += strspn(p, WHITESPACE); + p[strcspn(p, WHITESPACE)] = 0; + + return safe_atoi(p, mnt_id); +} + +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; - _cleanup_free_ char *parent = NULL; + bool nosupp = false, check_st_dev = true; struct stat a, b; int r; - bool nosupp = false; - /* We are not actually interested in the file handles, but - * name_to_handle_at() also passes us the mount ID, hence use - * it but throw the handle away */ + assert(fd >= 0); + assert(filename); - if (path_equal(t, "/")) - return 1; - - r = name_to_handle_at(AT_FDCWD, t, &h.handle, &mount_id, allow_symlink ? AT_SYMLINK_FOLLOW : 0); + /* First we will try the name_to_handle_at() syscall, which + * tells us the mount id and an opaque file "handle". It is + * not supported everywhere though (kernel compile-time + * option, not all file systems are hooked up). If it works + * the mount id is usually good enough to tell us whether + * something is a mount point. + * + * If that didn't work we will try to read the mount id from + * /proc/self/fdinfo/. This is almost as good as + * name_to_handle_at(), however, does not return the + * opaque file handle. The opaque file handle is pretty useful + * to detect the root directory, which we should always + * consider a mount point. Hence we use this only as + * fallback. Exporting the mnt_id in fdinfo is a pretty recent + * kernel addition. + * + * As last fallback we do traditional fstat() based st_dev + * comparisons. This is how things were traditionally done, + * but unionfs breaks breaks this since it exposes file + * systems with a variety of st_dev reported. Also, btrfs + * subvolumes have different st_dev, even though they aren't + * real mounts of their own. */ + + 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() - * fall back to the traditional stat() logic. */ - goto fallback; + * fall back to simpler logic. */ + goto fallback_fdinfo; else if (errno == EOPNOTSUPP) /* This kernel or file system does not support - * name_to_handle_at(), hence fallback to the + * name_to_handle_at(), hence let's see if the + * upper fs supports it (in which case it is a + * mount point), otherwise fallback to the * traditional stat() logic */ nosupp = true; - else if (errno == ENOENT) - return 0; else return -errno; } - r = path_get_parent(t, &parent); - if (r < 0) - return r; - - h.handle.handle_bytes = MAX_HANDLE_SZ; - r = name_to_handle_at(AT_FDCWD, parent, &h.handle, &mount_id_parent, AT_SYMLINK_FOLLOW); - if (r < 0) - if (errno == EOPNOTSUPP) + r = name_to_handle_at(fd, "", &h_parent.handle, &mount_id_parent, AT_EMPTY_PATH); + if (r < 0) { + if (errno == EOPNOTSUPP) { if (nosupp) /* Neither parent nor child do name_to_handle_at()? We have no choice but to fall back. */ - goto fallback; + goto fallback_fdinfo; else - /* The parent can't do name_to_handle_at() but - * the directory we are interested in can? - * Or the other way around? + /* The parent can't do name_to_handle_at() but the + * directory we are interested in can? * If so, it must be a mount point. */ return 1; - else + } else return -errno; - else - return mount_id != mount_id_parent; + } -fallback: - if (allow_symlink) - r = stat(t, &a); - else - r = lstat(t, &a); + /* The parent can do name_to_handle_at() but the + * directory we are interested in can't? If so, it + * must be a mount point. */ + if (nosupp) + return 1; - if (r < 0) { - if (errno == ENOENT) - return 0; + /* If the file handle for the directory we are + * interested in and its parent are identical, we + * assume this is the root directory, which is a mount + * point. */ - return -errno; - } + if (h.handle.handle_bytes == h_parent.handle.handle_bytes && + h.handle.handle_type == h_parent.handle.handle_type && + memcmp(h.handle.f_handle, h_parent.handle.f_handle, h.handle.handle_bytes) == 0) + return 1; - free(parent); - parent = NULL; + return mount_id != mount_id_parent; - r = path_get_parent(t, &parent); +fallback_fdinfo: + r = fd_fdinfo_mnt_id(fd, filename, flags, &mount_id); + if (r == -EOPNOTSUPP) + goto fallback_fstat; if (r < 0) return r; - r = stat(parent, &b); + r = fd_fdinfo_mnt_id(fd, "", AT_EMPTY_PATH, &mount_id_parent); if (r < 0) + return r; + + if (mount_id != mount_id_parent) + return 1; + + /* Hmm, so, the mount ids are the same. This leaves one + * special case though for the root file system. For that, + * let's see if the parent directory has the same inode as we + * are interested in. Hence, let's also do fstat() checks now, + * too, but avoid the st_dev comparisons, since they aren't + * that useful on unionfs mounts. */ + check_st_dev = false; + +fallback_fstat: + /* 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, AT_EMPTY_PATH) < 0) + return -errno; + + /* A directory with same device and inode as its parent? Must + * be the root directory */ + if (a.st_dev == b.st_dev && + a.st_ino == b.st_ino) + return 1; + + return check_st_dev && (a.st_dev != b.st_dev); +} + +/* flags can be AT_SYMLINK_FOLLOW or 0 */ +int path_is_mount_point(const char *t, int flags) { + _cleanup_close_ int fd = -1; + _cleanup_free_ char *canonical = NULL, *parent = NULL; + + assert(t); + + if (path_equal(t, "/")) + return 1; + + /* we need to resolve symlinks manually, we can't just rely on + * fd_is_mount_point() to do that for us; if we have a structure like + * /bin -> /usr/bin/ and /usr is a mount point, then the parent that we + * look at needs to be /usr, not /. */ + if (flags & AT_SYMLINK_FOLLOW) { + canonical = canonicalize_file_name(t); + if (!canonical) + return -errno; + + t = canonical; + } + + parent = dirname_malloc(t); + if (!parent) + return -ENOMEM; + + fd = openat(AT_FDCWD, parent, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_PATH); + if (fd < 0) return -errno; - return a.st_dev != b.st_dev; + return fd_is_mount_point(fd, basename(t), flags); } int path_is_read_only_fs(const char *path) { diff --git a/src/shared/path-util.h b/src/shared/path-util.h index 71bb740e9..e16484087 100644 --- a/src/shared/path-util.h +++ b/src/shared/path-util.h @@ -53,7 +53,8 @@ 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 path_is_mount_point(const char *path, bool allow_symlink); +int fd_is_mount_point(int fd, const char *filename, int flags); +int path_is_mount_point(const char *path, int flags); int path_is_read_only_fs(const char *path); int path_is_os_tree(const char *path); diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 6396fcb39..8870f178a 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -21,6 +21,7 @@ #include #include +#include #include "path-util.h" #include "util.h" @@ -85,8 +86,8 @@ static void test_path(void) { test_parent("/aa///file...", "/aa///"); test_parent("file.../", NULL); - assert_se(path_is_mount_point("/", true)); - assert_se(path_is_mount_point("/", false)); + assert_se(path_is_mount_point("/", AT_SYMLINK_FOLLOW)); + assert_se(path_is_mount_point("/", 0)); { char p1[] = "aaa/bbb////ccc"; @@ -99,6 +100,66 @@ static void test_path(void) { } } +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("/", AT_SYMLINK_FOLLOW) > 0); + assert_se(path_is_mount_point("/", 0) > 0); + + assert_se(path_is_mount_point("/proc", AT_SYMLINK_FOLLOW) > 0); + assert_se(path_is_mount_point("/proc", 0) > 0); + + assert_se(path_is_mount_point("/proc/1", AT_SYMLINK_FOLLOW) == 0); + assert_se(path_is_mount_point("/proc/1", 0) == 0); + + assert_se(path_is_mount_point("/sys", AT_SYMLINK_FOLLOW) > 0); + assert_se(path_is_mount_point("/sys", 0) > 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, AT_SYMLINK_FOLLOW) == 0); + assert_se(path_is_mount_point(file1, 0) == 0); + assert_se(path_is_mount_point(link1, AT_SYMLINK_FOLLOW) == 0); + assert_se(path_is_mount_point(link1, 0) == 0); + + /* this test will only work as root */ + if (mount(file1, file2, NULL, MS_BIND, NULL) >= 0) { + rf = path_is_mount_point(file2, 0); + rt = path_is_mount_point(file2, AT_SYMLINK_FOLLOW); + rlf = path_is_mount_point(link2, 0); + rlt = path_is_mount_point(link2, AT_SYMLINK_FOLLOW); + + 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, false, true, false) == 0); +} + static void test_find_binary(const char *self, bool local) { char *p; @@ -288,6 +349,7 @@ int main(int argc, char **argv) { test_make_relative(); test_strv_resolve(); test_path_startswith(); + test_path_is_mount_point(); return 0; }