diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c index 5f22bd68ce..e32502308a 100644 --- a/src/basic/mount-util.c +++ b/src/basic/mount-util.c @@ -65,7 +65,6 @@ int name_to_handle_at_loop( for (;;) { int mnt_id = -1; - size_t m; if (name_to_handle_at(fd, path, h, &mnt_id, flags) >= 0) { @@ -84,22 +83,24 @@ int name_to_handle_at_loop( if (!ret_handle && ret_mnt_id && mnt_id >= 0) { - /* As it appears, name_to_handle_at() fills in mnt_id even when it returns EOVERFLOW, but - * that's undocumented. Hence, let's make use of this if it appears to be filled in, and the - * caller was interested in only the mount ID an nothing else. */ + /* As it appears, name_to_handle_at() fills in mnt_id even when it returns EOVERFLOW when the + * buffer is too small, but that's undocumented. Hence, let's make use of this if it appears to + * be filled in, and the caller was interested in only the mount ID an nothing else. */ *ret_mnt_id = mnt_id; return 0; } - /* The buffer was too small. Size the new buffer by what name_to_handle_at() returned, but make sure it - * is always larger than what we passed in before */ - m = h->handle_bytes > n ? h->handle_bytes : n * 2; - if (m < n) /* Check for multiplication overflow */ + /* If name_to_handle_at() didn't increase the byte size, then this EOVERFLOW is caused by something + * else (apparently EOVERFLOW is returned for untriggered nfs4 mounts sometimes), not by the too small + * buffer. In that case propagate EOVERFLOW */ + if (h->handle_bytes <= n) return -EOVERFLOW; - if (offsetof(struct file_handle, f_handle) + m < m) /* check for addition overflow */ + + /* The buffer was too small. Size the new buffer by what name_to_handle_at() returned. */ + n = h->handle_bytes; + if (offsetof(struct file_handle, f_handle) + n < n) /* check for addition overflow */ return -EOVERFLOW; - n = m; free(h); h = malloc0(offsetof(struct file_handle, f_handle) + n); @@ -131,7 +132,7 @@ static int fd_fdinfo_mnt_id(int fd, const char *filename, int flags, int *mnt_id if (r == -ENOENT) /* The fdinfo directory is a relatively new addition */ return -EOPNOTSUPP; if (r < 0) - return -errno; + return r; p = startswith(fdinfo, "mnt_id:"); if (!p) { @@ -182,9 +183,10 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { * real mounts of their own. */ r = name_to_handle_at_loop(fd, filename, &h, &mount_id, flags); - if (IN_SET(r, -ENOSYS, -EACCES, -EPERM)) - /* This kernel does not support name_to_handle_at() at all, or the syscall was blocked (maybe through - * seccomp, because we are running inside of a container?): fall back to simpler logic. */ + if (IN_SET(r, -ENOSYS, -EACCES, -EPERM, -EOVERFLOW)) + /* This kernel does not support name_to_handle_at() at all (ENOSYS), or the syscall was blocked + * (EACCES/EPERM; maybe through seccomp, because we are running inside of a container?), or the mount + * point is not triggered yet (EOVERFLOW, thinkg nfs4): fall back to simpler logic. */ goto fallback_fdinfo; else if (r == -EOPNOTSUPP) /* This kernel or file system does not support name_to_handle_at(), hence let's see if the upper fs @@ -204,7 +206,7 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { * it must be a mount point. */ return 1; } else if (r < 0) - return r; + return r; /* The parent can do name_to_handle_at() but the * directory we are interested in can't? If so, it @@ -306,7 +308,7 @@ int path_get_mnt_id(const char *path, int *ret) { int r; r = name_to_handle_at_loop(AT_FDCWD, path, NULL, ret, 0); - if (IN_SET(r, -EOPNOTSUPP, -ENOSYS, -EACCES, -EPERM)) /* kernel/fs don't support this, or seccomp blocks access */ + if (IN_SET(r, -EOPNOTSUPP, -ENOSYS, -EACCES, -EPERM, -EOVERFLOW)) /* kernel/fs don't support this, or seccomp blocks access, or untriggered mount */ return fd_fdinfo_mnt_id(AT_FDCWD, path, 0, ret); return r; diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c index 5a93da1c0d..9582d6ceec 100644 --- a/src/test/test-mount-util.c +++ b/src/test/test-mount-util.c @@ -27,6 +27,7 @@ #include "hashmap.h" #include "log.h" #include "mount-util.h" +#include "path-util.h" #include "string-util.h" static void test_mount_propagation_flags(const char *name, int ret, unsigned long expected) { @@ -51,16 +52,15 @@ static void test_mnt_id(void) { _cleanup_fclose_ FILE *f = NULL; Hashmap *h; Iterator i; - char *k; - void *p; + char *p; + void *k; int r; assert_se(f = fopen("/proc/self/mountinfo", "re")); - assert_se(h = hashmap_new(&string_hash_ops)); + assert_se(h = hashmap_new(&trivial_hash_ops)); for (;;) { _cleanup_free_ char *line = NULL, *path = NULL; - void *old_key; int mnt_id; r = read_line(f, LONG_LINE_MAX, &line); @@ -70,38 +70,35 @@ static void test_mnt_id(void) { assert_se(sscanf(line, "%i %*s %*s %*s %ms", &mnt_id, &path) == 2); - /* Add all mount points and their ids to a hashtable, so that we filter out mount points that are - * overmounted. For those we only care for the "upper" mount, since that's the only one - * path_get_mnt_id() can determine. */ - - if (hashmap_remove2(h, path, &old_key)) - free(old_key); - - assert_se(hashmap_put(h, path, INT_TO_PTR(mnt_id)) >= 0); + assert_se(hashmap_put(h, INT_TO_PTR(mnt_id), path) >= 0); path = NULL; } HASHMAP_FOREACH_KEY(p, k, h, i) { - int mnt_id = PTR_TO_INT(p), mnt_id2; + int mnt_id = PTR_TO_INT(k), mnt_id2; - r = path_get_mnt_id(k, &mnt_id2); + r = path_get_mnt_id(p, &mnt_id2); if (r == -EOPNOTSUPP) { /* kernel or file system too old? */ - log_debug("%s doesn't support mount IDs\n", k); + log_debug("%s doesn't support mount IDs\n", p); continue; } if (IN_SET(r, -EACCES, -EPERM)) { - log_debug("Can't access %s\n", k); + log_debug("Can't access %s\n", p); continue; } - log_debug("mnt id of %s is %i\n", k, mnt_id2); + log_debug("mnt id of %s is %i\n", p, mnt_id2); - assert_se(r >= 0); - assert_se(mnt_id == mnt_id2); + if (mnt_id == mnt_id2) + continue; + + /* The ids don't match? If so, then there are two mounts on the same path, let's check if that's really + * the case */ + assert_se(path_equal_ptr(hashmap_get(h, INT_TO_PTR(mnt_id2)), p)); } - while ((k = hashmap_steal_first_key(h))) - free(k); + while ((p = hashmap_steal_first(h))) + free(p); hashmap_free(h); }