* [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps
@ 2023-12-14 6:44 Andrei Vagin
2023-12-14 6:44 ` [PATCH 2/2] selftests/overlayfs: verify " Andrei Vagin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrei Vagin @ 2023-12-14 6:44 UTC (permalink / raw)
To: Andrew Morton, Amir Goldstein
Cc: linux-kernel, Alexander Mikhalitsyn, Christian Brauner,
Alexander Viro, linux-fsdevel, overlayfs, Miklos Szeredi,
Andrei Vagin
/proc/pid/maps shows device and inode numbers of vma->vm_file-s. Here is
an issue. If a mapped file is on a stackable file system (e.g.,
overlayfs), vma->vm_file is a backing file whose f_inode is on the
underlying filesystem. To show correct numbers, we need to get a user
file and shows its numbers. The same trick is used to show file paths in
/proc/pid/maps.
Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrei Vagin <avagin@google.com>
---
v2: Amir explained that vfs_getattr isn't needed, because
file_user_inode(vma->vm_file).i_ino always matches an inode number
returned by statx.
fs/proc/task_mmu.c | 3 ++-
include/linux/fs.h | 18 +++++++++++++-----
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 435b61054b5b..1801e409a061 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -273,7 +273,8 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
const char *name = NULL;
if (file) {
- struct inode *inode = file_inode(vma->vm_file);
+ const struct inode *inode = file_user_inode(vma->vm_file);
+
dev = inode->i_sb->s_dev;
ino = inode->i_ino;
pgoff = ((loff_t)vma->vm_pgoff) << PAGE_SHIFT;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 98b7a7a8c42e..838ccfc63323 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2523,20 +2523,28 @@ struct file *backing_file_open(const struct path *user_path, int flags,
struct path *backing_file_user_path(struct file *f);
/*
- * file_user_path - get the path to display for memory mapped file
- *
* When mmapping a file on a stackable filesystem (e.g., overlayfs), the file
* stored in ->vm_file is a backing file whose f_inode is on the underlying
- * filesystem. When the mapped file path is displayed to user (e.g. via
- * /proc/<pid>/maps), this helper should be used to get the path to display
- * to the user, which is the path of the fd that user has requested to map.
+ * filesystem. When the mapped file path and inode number are displayed to
+ * user (e.g. via /proc/<pid>/maps), these helpers should be used to get the
+ * path and inode number to display to the user, which is the path of the fd
+ * that user has requested to map and the inode number that would be returned
+ * by fstat() on that same fd.
*/
+/* Get the path to display in /proc/<pid>/maps */
static inline const struct path *file_user_path(struct file *f)
{
if (unlikely(f->f_mode & FMODE_BACKING))
return backing_file_user_path(f);
return &f->f_path;
}
+/* Get the inode whose inode number to display in /proc/<pid>/maps */
+static inline const struct inode *file_user_inode(struct file *f)
+{
+ if (unlikely(f->f_mode & FMODE_BACKING))
+ return d_inode(backing_file_user_path(f)->dentry);
+ return file_inode(f);
+}
static inline struct file *file_clone_open(struct file *file)
{
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] selftests/overlayfs: verify device and inode numbers in /proc/pid/maps 2023-12-14 6:44 [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps Andrei Vagin @ 2023-12-14 6:44 ` Andrei Vagin 2023-12-14 8:03 ` Amir Goldstein 2023-12-14 8:11 ` [PATCH 1/2 v2] fs/proc: show correct " Amir Goldstein 2023-12-14 10:55 ` Christian Brauner 2 siblings, 1 reply; 6+ messages in thread From: Andrei Vagin @ 2023-12-14 6:44 UTC (permalink / raw) To: Andrew Morton, Amir Goldstein Cc: linux-kernel, Alexander Mikhalitsyn, Christian Brauner, Alexander Viro, linux-fsdevel, overlayfs, Miklos Szeredi, Andrei Vagin When mapping a file on overlayfs, the file stored in ->vm_file is a backing file whose f_inode is on the underlying filesystem. We need to verify that /proc/pid/maps contains numbers of the overlayfs file, but not its backing file. Cc: Amir Goldstein <amir73il@gmail.com> Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com> Signed-off-by: Andrei Vagin <avagin@google.com> --- tools/testing/selftests/Makefile | 1 + .../filesystems/overlayfs/.gitignore | 2 + .../selftests/filesystems/overlayfs/Makefile | 7 + .../filesystems/overlayfs/dev_in_maps.c | 182 ++++++++++++++++++ .../selftests/filesystems/overlayfs/log.h | 26 +++ 5 files changed, 218 insertions(+) create mode 100644 tools/testing/selftests/filesystems/overlayfs/.gitignore create mode 100644 tools/testing/selftests/filesystems/overlayfs/Makefile create mode 100644 tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c create mode 100644 tools/testing/selftests/filesystems/overlayfs/log.h diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 3b2061d1c1a5..0939a40abb28 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -26,6 +26,7 @@ TARGETS += filesystems TARGETS += filesystems/binderfs TARGETS += filesystems/epoll TARGETS += filesystems/fat +TARGETS += filesystems/overlayfs TARGETS += firmware TARGETS += fpu TARGETS += ftrace diff --git a/tools/testing/selftests/filesystems/overlayfs/.gitignore b/tools/testing/selftests/filesystems/overlayfs/.gitignore new file mode 100644 index 000000000000..52ae618fdd98 --- /dev/null +++ b/tools/testing/selftests/filesystems/overlayfs/.gitignore @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +dev_in_maps diff --git a/tools/testing/selftests/filesystems/overlayfs/Makefile b/tools/testing/selftests/filesystems/overlayfs/Makefile new file mode 100644 index 000000000000..56b2b48a765b --- /dev/null +++ b/tools/testing/selftests/filesystems/overlayfs/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 + +TEST_GEN_PROGS := dev_in_maps + +CFLAGS := -Wall -Werror + +include ../../lib.mk diff --git a/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c new file mode 100644 index 000000000000..e19ab0e85709 --- /dev/null +++ b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE + +#include <inttypes.h> +#include <unistd.h> +#include <stdio.h> + +#include <linux/unistd.h> +#include <linux/types.h> +#include <linux/mount.h> +#include <sys/syscall.h> +#include <sys/stat.h> +#include <sys/mount.h> +#include <sys/mman.h> +#include <sched.h> +#include <fcntl.h> + +#include "../../kselftest.h" +#include "log.h" + +static int sys_fsopen(const char *fsname, unsigned int flags) +{ + return syscall(__NR_fsopen, fsname, flags); +} + +static int sys_fsconfig(int fd, unsigned int cmd, const char *key, const char *value, int aux) +{ + return syscall(__NR_fsconfig, fd, cmd, key, value, aux); +} + +static int sys_fsmount(int fd, unsigned int flags, unsigned int attr_flags) +{ + return syscall(__NR_fsmount, fd, flags, attr_flags); +} + +static int sys_move_mount(int from_dfd, const char *from_pathname, + int to_dfd, const char *to_pathname, + unsigned int flags) +{ + return syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, to_pathname, flags); +} + +static long get_file_dev_and_inode(void *addr, struct statx *stx) +{ + char buf[4096]; + FILE *mapf; + + mapf = fopen("/proc/self/maps", "r"); + if (mapf == NULL) + return pr_perror("fopen(/proc/self/maps)"); + + while (fgets(buf, sizeof(buf), mapf)) { + unsigned long start, end; + uint32_t maj, min; + __u64 ino; + + if (sscanf(buf, "%lx-%lx %*s %*s %x:%x %llu", + &start, &end, &maj, &min, &ino) != 5) + return pr_perror("unable to parse: %s", buf); + if (start == (unsigned long)addr) { + stx->stx_dev_major = maj; + stx->stx_dev_minor = min; + stx->stx_ino = ino; + return 0; + } + } + + return pr_err("unable to find the mapping"); +} + +static int ovl_mount(void) +{ + int tmpfs, fsfd, ovl; + + fsfd = sys_fsopen("tmpfs", 0); + if (fsfd == -1) + return pr_perror("fsopen(tmpfs)"); + + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) == -1) + return pr_perror("FSCONFIG_CMD_CREATE"); + + tmpfs = sys_fsmount(fsfd, 0, 0); + if (tmpfs == -1) + return pr_perror("fsmount"); + + close(fsfd); + + /* overlayfs can't be constructed on top of a detached mount. */ + if (sys_move_mount(tmpfs, "", AT_FDCWD, "/tmp", MOVE_MOUNT_F_EMPTY_PATH)) + return pr_perror("move_mount"); + close(tmpfs); + + if (mkdir("/tmp/w", 0755) == -1 || + mkdir("/tmp/u", 0755) == -1 || + mkdir("/tmp/l", 0755) == -1) + return pr_perror("mkdir"); + + fsfd = sys_fsopen("overlay", 0); + if (fsfd == -1) + return pr_perror("fsopen(overlay)"); + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", "test", 0) == -1 || + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "lowerdir", "/tmp/l", 0) == -1 || + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "upperdir", "/tmp/u", 0) == -1 || + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "workdir", "/tmp/w", 0) == -1) + return pr_perror("fsconfig"); + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) == -1) + return pr_perror("fsconfig"); + ovl = sys_fsmount(fsfd, 0, 0); + if (ovl == -1) + return pr_perror("fsmount"); + + return ovl; +} + +/* + * Check that the file device and inode shown in /proc/pid/maps match values + * returned by stat(2). + */ +static int test(void) +{ + struct statx stx, mstx; + int ovl, fd; + void *addr; + + ovl = ovl_mount(); + if (ovl == -1) + return -1; + + fd = openat(ovl, "test", O_RDWR | O_CREAT, 0644); + if (fd == -1) + return pr_perror("openat"); + + addr = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0); + if (addr == MAP_FAILED) + return pr_perror("mmap"); + + if (get_file_dev_and_inode(addr, &mstx)) + return -1; + if (statx(fd, "", AT_EMPTY_PATH | AT_STATX_SYNC_AS_STAT, STATX_INO, &stx)) + return pr_perror("statx"); + + if (stx.stx_dev_major != mstx.stx_dev_major || + stx.stx_dev_minor != mstx.stx_dev_minor || + stx.stx_ino != mstx.stx_ino) + return pr_fail("unmatched dev:ino %x:%x:%llx (expected %x:%x:%llx)\n", + mstx.stx_dev_major, mstx.stx_dev_minor, mstx.stx_ino, + stx.stx_dev_major, stx.stx_dev_minor, stx.stx_ino); + + ksft_test_result_pass("devices are matched\n"); + return 0; +} + +int main(int argc, char **argv) +{ + int fsfd; + + fsfd = sys_fsopen("overlay", 0); + if (fsfd == -1) { + ksft_test_result_skip("unable to create overlay mount\n"); + return 1; + } + close(fsfd); + + /* Create a new mount namespace to not care about cleaning test mounts. */ + if (unshare(CLONE_NEWNS) == -1) { + ksft_test_result_skip("unable to create a new mount namespace\n"); + return 1; + } + + if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) == -1) { + pr_perror("mount"); + return 1; + } + + ksft_set_plan(1); + + if (test()) + return 1; + + ksft_exit_pass(); + return 0; +} diff --git a/tools/testing/selftests/filesystems/overlayfs/log.h b/tools/testing/selftests/filesystems/overlayfs/log.h new file mode 100644 index 000000000000..db64df2a8483 --- /dev/null +++ b/tools/testing/selftests/filesystems/overlayfs/log.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __SELFTEST_TIMENS_LOG_H__ +#define __SELFTEST_TIMENS_LOG_H__ + +#define pr_msg(fmt, lvl, ...) \ + ksft_print_msg("[%s] (%s:%d)\t" fmt "\n", \ + lvl, __FILE__, __LINE__, ##__VA_ARGS__) + +#define pr_p(func, fmt, ...) func(fmt ": %m", ##__VA_ARGS__) + +#define pr_err(fmt, ...) \ + ({ \ + ksft_test_result_error(fmt "\n", ##__VA_ARGS__); \ + -1; \ + }) + +#define pr_fail(fmt, ...) \ + ({ \ + ksft_test_result_fail(fmt, ##__VA_ARGS__); \ + -1; \ + }) + +#define pr_perror(fmt, ...) pr_p(pr_err, fmt, ##__VA_ARGS__) + +#endif -- 2.43.0.472.g3155946c3a-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] selftests/overlayfs: verify device and inode numbers in /proc/pid/maps 2023-12-14 6:44 ` [PATCH 2/2] selftests/overlayfs: verify " Andrei Vagin @ 2023-12-14 8:03 ` Amir Goldstein 0 siblings, 0 replies; 6+ messages in thread From: Amir Goldstein @ 2023-12-14 8:03 UTC (permalink / raw) To: Andrei Vagin Cc: Andrew Morton, linux-kernel, Alexander Mikhalitsyn, Christian Brauner, Alexander Viro, linux-fsdevel, overlayfs, Miklos Szeredi On Thu, Dec 14, 2023 at 8:44 AM Andrei Vagin <avagin@google.com> wrote: > > When mapping a file on overlayfs, the file stored in ->vm_file is a > backing file whose f_inode is on the underlying filesystem. We need to > verify that /proc/pid/maps contains numbers of the overlayfs file, but > not its backing file. > > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com> > Signed-off-by: Andrei Vagin <avagin@google.com> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > tools/testing/selftests/Makefile | 1 + > .../filesystems/overlayfs/.gitignore | 2 + > .../selftests/filesystems/overlayfs/Makefile | 7 + > .../filesystems/overlayfs/dev_in_maps.c | 182 ++++++++++++++++++ > .../selftests/filesystems/overlayfs/log.h | 26 +++ > 5 files changed, 218 insertions(+) > create mode 100644 tools/testing/selftests/filesystems/overlayfs/.gitignore > create mode 100644 tools/testing/selftests/filesystems/overlayfs/Makefile > create mode 100644 tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c > create mode 100644 tools/testing/selftests/filesystems/overlayfs/log.h > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 3b2061d1c1a5..0939a40abb28 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -26,6 +26,7 @@ TARGETS += filesystems > TARGETS += filesystems/binderfs > TARGETS += filesystems/epoll > TARGETS += filesystems/fat > +TARGETS += filesystems/overlayfs > TARGETS += firmware > TARGETS += fpu > TARGETS += ftrace > diff --git a/tools/testing/selftests/filesystems/overlayfs/.gitignore b/tools/testing/selftests/filesystems/overlayfs/.gitignore > new file mode 100644 > index 000000000000..52ae618fdd98 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/.gitignore > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +dev_in_maps > diff --git a/tools/testing/selftests/filesystems/overlayfs/Makefile b/tools/testing/selftests/filesystems/overlayfs/Makefile > new file mode 100644 > index 000000000000..56b2b48a765b > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/Makefile > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +TEST_GEN_PROGS := dev_in_maps > + > +CFLAGS := -Wall -Werror > + > +include ../../lib.mk > diff --git a/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c > new file mode 100644 > index 000000000000..e19ab0e85709 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#define _GNU_SOURCE > + > +#include <inttypes.h> > +#include <unistd.h> > +#include <stdio.h> > + > +#include <linux/unistd.h> > +#include <linux/types.h> > +#include <linux/mount.h> > +#include <sys/syscall.h> > +#include <sys/stat.h> > +#include <sys/mount.h> > +#include <sys/mman.h> > +#include <sched.h> > +#include <fcntl.h> > + > +#include "../../kselftest.h" > +#include "log.h" > + > +static int sys_fsopen(const char *fsname, unsigned int flags) > +{ > + return syscall(__NR_fsopen, fsname, flags); > +} > + > +static int sys_fsconfig(int fd, unsigned int cmd, const char *key, const char *value, int aux) > +{ > + return syscall(__NR_fsconfig, fd, cmd, key, value, aux); > +} > + > +static int sys_fsmount(int fd, unsigned int flags, unsigned int attr_flags) > +{ > + return syscall(__NR_fsmount, fd, flags, attr_flags); > +} > + > +static int sys_move_mount(int from_dfd, const char *from_pathname, > + int to_dfd, const char *to_pathname, > + unsigned int flags) > +{ > + return syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, to_pathname, flags); > +} > + > +static long get_file_dev_and_inode(void *addr, struct statx *stx) > +{ > + char buf[4096]; > + FILE *mapf; > + > + mapf = fopen("/proc/self/maps", "r"); > + if (mapf == NULL) > + return pr_perror("fopen(/proc/self/maps)"); > + > + while (fgets(buf, sizeof(buf), mapf)) { > + unsigned long start, end; > + uint32_t maj, min; > + __u64 ino; > + > + if (sscanf(buf, "%lx-%lx %*s %*s %x:%x %llu", > + &start, &end, &maj, &min, &ino) != 5) > + return pr_perror("unable to parse: %s", buf); > + if (start == (unsigned long)addr) { > + stx->stx_dev_major = maj; > + stx->stx_dev_minor = min; > + stx->stx_ino = ino; > + return 0; > + } > + } > + > + return pr_err("unable to find the mapping"); > +} > + > +static int ovl_mount(void) > +{ > + int tmpfs, fsfd, ovl; > + > + fsfd = sys_fsopen("tmpfs", 0); > + if (fsfd == -1) > + return pr_perror("fsopen(tmpfs)"); > + > + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) == -1) > + return pr_perror("FSCONFIG_CMD_CREATE"); > + > + tmpfs = sys_fsmount(fsfd, 0, 0); > + if (tmpfs == -1) > + return pr_perror("fsmount"); > + > + close(fsfd); > + > + /* overlayfs can't be constructed on top of a detached mount. */ > + if (sys_move_mount(tmpfs, "", AT_FDCWD, "/tmp", MOVE_MOUNT_F_EMPTY_PATH)) > + return pr_perror("move_mount"); > + close(tmpfs); > + > + if (mkdir("/tmp/w", 0755) == -1 || > + mkdir("/tmp/u", 0755) == -1 || > + mkdir("/tmp/l", 0755) == -1) > + return pr_perror("mkdir"); > + > + fsfd = sys_fsopen("overlay", 0); > + if (fsfd == -1) > + return pr_perror("fsopen(overlay)"); > + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", "test", 0) == -1 || > + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "lowerdir", "/tmp/l", 0) == -1 || > + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "upperdir", "/tmp/u", 0) == -1 || > + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "workdir", "/tmp/w", 0) == -1) > + return pr_perror("fsconfig"); > + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) == -1) > + return pr_perror("fsconfig"); > + ovl = sys_fsmount(fsfd, 0, 0); > + if (ovl == -1) > + return pr_perror("fsmount"); > + > + return ovl; > +} > + > +/* > + * Check that the file device and inode shown in /proc/pid/maps match values > + * returned by stat(2). > + */ > +static int test(void) > +{ > + struct statx stx, mstx; > + int ovl, fd; > + void *addr; > + > + ovl = ovl_mount(); > + if (ovl == -1) > + return -1; > + > + fd = openat(ovl, "test", O_RDWR | O_CREAT, 0644); > + if (fd == -1) > + return pr_perror("openat"); > + > + addr = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0); > + if (addr == MAP_FAILED) > + return pr_perror("mmap"); > + > + if (get_file_dev_and_inode(addr, &mstx)) > + return -1; > + if (statx(fd, "", AT_EMPTY_PATH | AT_STATX_SYNC_AS_STAT, STATX_INO, &stx)) > + return pr_perror("statx"); > + > + if (stx.stx_dev_major != mstx.stx_dev_major || > + stx.stx_dev_minor != mstx.stx_dev_minor || > + stx.stx_ino != mstx.stx_ino) > + return pr_fail("unmatched dev:ino %x:%x:%llx (expected %x:%x:%llx)\n", > + mstx.stx_dev_major, mstx.stx_dev_minor, mstx.stx_ino, > + stx.stx_dev_major, stx.stx_dev_minor, stx.stx_ino); > + > + ksft_test_result_pass("devices are matched\n"); > + return 0; > +} > + > +int main(int argc, char **argv) > +{ > + int fsfd; > + > + fsfd = sys_fsopen("overlay", 0); > + if (fsfd == -1) { > + ksft_test_result_skip("unable to create overlay mount\n"); > + return 1; > + } > + close(fsfd); > + > + /* Create a new mount namespace to not care about cleaning test mounts. */ > + if (unshare(CLONE_NEWNS) == -1) { > + ksft_test_result_skip("unable to create a new mount namespace\n"); > + return 1; > + } > + > + if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) == -1) { > + pr_perror("mount"); > + return 1; > + } > + > + ksft_set_plan(1); > + > + if (test()) > + return 1; > + > + ksft_exit_pass(); > + return 0; > +} > diff --git a/tools/testing/selftests/filesystems/overlayfs/log.h b/tools/testing/selftests/filesystems/overlayfs/log.h > new file mode 100644 > index 000000000000..db64df2a8483 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/log.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef __SELFTEST_TIMENS_LOG_H__ > +#define __SELFTEST_TIMENS_LOG_H__ > + > +#define pr_msg(fmt, lvl, ...) \ > + ksft_print_msg("[%s] (%s:%d)\t" fmt "\n", \ > + lvl, __FILE__, __LINE__, ##__VA_ARGS__) > + > +#define pr_p(func, fmt, ...) func(fmt ": %m", ##__VA_ARGS__) > + > +#define pr_err(fmt, ...) \ > + ({ \ > + ksft_test_result_error(fmt "\n", ##__VA_ARGS__); \ > + -1; \ > + }) > + > +#define pr_fail(fmt, ...) \ > + ({ \ > + ksft_test_result_fail(fmt, ##__VA_ARGS__); \ > + -1; \ > + }) > + > +#define pr_perror(fmt, ...) pr_p(pr_err, fmt, ##__VA_ARGS__) > + > +#endif > -- > 2.43.0.472.g3155946c3a-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps 2023-12-14 6:44 [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps Andrei Vagin 2023-12-14 6:44 ` [PATCH 2/2] selftests/overlayfs: verify " Andrei Vagin @ 2023-12-14 8:11 ` Amir Goldstein 2023-12-14 10:55 ` Christian Brauner 2 siblings, 0 replies; 6+ messages in thread From: Amir Goldstein @ 2023-12-14 8:11 UTC (permalink / raw) To: Andrei Vagin Cc: Andrew Morton, linux-kernel, Alexander Mikhalitsyn, Christian Brauner, Alexander Viro, linux-fsdevel, overlayfs, Miklos Szeredi On Thu, Dec 14, 2023 at 8:44 AM Andrei Vagin <avagin@google.com> wrote: > > /proc/pid/maps shows device and inode numbers of vma->vm_file-s. Here is > an issue. If a mapped file is on a stackable file system (e.g., > overlayfs), vma->vm_file is a backing file whose f_inode is on the > underlying filesystem. To show correct numbers, we need to get a user > file and shows its numbers. The same trick is used to show file paths in > /proc/pid/maps. > > Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com> > Suggested-by: Amir Goldstein <amir73il@gmail.com> > Signed-off-by: Andrei Vagin <avagin@google.com> > --- > v2: Amir explained that vfs_getattr isn't needed, because > file_user_inode(vma->vm_file).i_ino always matches an inode number > returned by statx. At least i_ino *should* always match st_ino for overlayfs non-dirs. If it doesn't, it is a bug. Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks, Amir. > > fs/proc/task_mmu.c | 3 ++- > include/linux/fs.h | 18 +++++++++++++----- > 2 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index 435b61054b5b..1801e409a061 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -273,7 +273,8 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma) > const char *name = NULL; > > if (file) { > - struct inode *inode = file_inode(vma->vm_file); > + const struct inode *inode = file_user_inode(vma->vm_file); > + > dev = inode->i_sb->s_dev; > ino = inode->i_ino; > pgoff = ((loff_t)vma->vm_pgoff) << PAGE_SHIFT; > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 98b7a7a8c42e..838ccfc63323 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2523,20 +2523,28 @@ struct file *backing_file_open(const struct path *user_path, int flags, > struct path *backing_file_user_path(struct file *f); > > /* > - * file_user_path - get the path to display for memory mapped file > - * > * When mmapping a file on a stackable filesystem (e.g., overlayfs), the file > * stored in ->vm_file is a backing file whose f_inode is on the underlying > - * filesystem. When the mapped file path is displayed to user (e.g. via > - * /proc/<pid>/maps), this helper should be used to get the path to display > - * to the user, which is the path of the fd that user has requested to map. > + * filesystem. When the mapped file path and inode number are displayed to > + * user (e.g. via /proc/<pid>/maps), these helpers should be used to get the > + * path and inode number to display to the user, which is the path of the fd > + * that user has requested to map and the inode number that would be returned > + * by fstat() on that same fd. > */ > +/* Get the path to display in /proc/<pid>/maps */ > static inline const struct path *file_user_path(struct file *f) > { > if (unlikely(f->f_mode & FMODE_BACKING)) > return backing_file_user_path(f); > return &f->f_path; > } > +/* Get the inode whose inode number to display in /proc/<pid>/maps */ > +static inline const struct inode *file_user_inode(struct file *f) > +{ > + if (unlikely(f->f_mode & FMODE_BACKING)) > + return d_inode(backing_file_user_path(f)->dentry); > + return file_inode(f); > +} > > static inline struct file *file_clone_open(struct file *file) > { > -- > 2.43.0.472.g3155946c3a-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps 2023-12-14 6:44 [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps Andrei Vagin 2023-12-14 6:44 ` [PATCH 2/2] selftests/overlayfs: verify " Andrei Vagin 2023-12-14 8:11 ` [PATCH 1/2 v2] fs/proc: show correct " Amir Goldstein @ 2023-12-14 10:55 ` Christian Brauner 2 siblings, 0 replies; 6+ messages in thread From: Christian Brauner @ 2023-12-14 10:55 UTC (permalink / raw) To: Andrei Vagin Cc: Christian Brauner, linux-kernel, Alexander Mikhalitsyn, Alexander Viro, linux-fsdevel, overlayfs, Miklos Szeredi, Andrew Morton, Amir Goldstein On Wed, 13 Dec 2023 22:44:38 -0800, Andrei Vagin wrote: > /proc/pid/maps shows device and inode numbers of vma->vm_file-s. Here is > an issue. If a mapped file is on a stackable file system (e.g., > overlayfs), vma->vm_file is a backing file whose f_inode is on the > underlying filesystem. To show correct numbers, we need to get a user > file and shows its numbers. The same trick is used to show file paths in > /proc/pid/maps. > > [...] Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.misc [1/2] fs/proc: show correct device and inode numbers in /proc/pid/maps https://git.kernel.org/vfs/vfs/c/26b50595e169 [2/2] selftests/overlayfs: verify device and inode numbers in /proc/pid/maps https://git.kernel.org/vfs/vfs/c/22d9cfff4639 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20231211193048.580691-1-avagin@google.com>]
[parent not found: <20231211193048.580691-2-avagin@google.com>]
* Re: [PATCH 2/2] selftests/overlayfs: verify device and inode numbers in /proc/pid/maps [not found] ` <20231211193048.580691-2-avagin@google.com> @ 2023-12-12 5:17 ` Amir Goldstein 0 siblings, 0 replies; 6+ messages in thread From: Amir Goldstein @ 2023-12-12 5:17 UTC (permalink / raw) To: Andrei Vagin Cc: Andrew Morton, linux-kernel, Alexander Mikhalitsyn, linux-fsdevel, Christian Brauner +fsdevel, +brauner On Mon, Dec 11, 2023 at 9:30 PM Andrei Vagin <avagin@google.com> wrote: > > When mapping a file on overlayfs, the file stored in ->vm_file is a > backing file whose f_inode is on the underlying filesystem. We need to > verify that /proc/pid/maps contains numbers of the overlayfs file, but > not its backing file. > > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com> > Signed-off-by: Andrei Vagin <avagin@google.com> Thanks for the fix and test. Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > tools/testing/selftests/Makefile | 1 + > .../filesystems/overlayfs/.gitignore | 2 + > .../selftests/filesystems/overlayfs/Makefile | 7 + > .../filesystems/overlayfs/dev_in_maps.c | 182 ++++++++++++++++++ > .../selftests/filesystems/overlayfs/log.h | 26 +++ > 5 files changed, 218 insertions(+) > create mode 100644 tools/testing/selftests/filesystems/overlayfs/.gitignore > create mode 100644 tools/testing/selftests/filesystems/overlayfs/Makefile > create mode 100644 tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c > create mode 100644 tools/testing/selftests/filesystems/overlayfs/log.h > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 3b2061d1c1a5..0939a40abb28 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -26,6 +26,7 @@ TARGETS += filesystems > TARGETS += filesystems/binderfs > TARGETS += filesystems/epoll > TARGETS += filesystems/fat > +TARGETS += filesystems/overlayfs > TARGETS += firmware > TARGETS += fpu > TARGETS += ftrace > diff --git a/tools/testing/selftests/filesystems/overlayfs/.gitignore b/tools/testing/selftests/filesystems/overlayfs/.gitignore > new file mode 100644 > index 000000000000..52ae618fdd98 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/.gitignore > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +dev_in_maps > diff --git a/tools/testing/selftests/filesystems/overlayfs/Makefile b/tools/testing/selftests/filesystems/overlayfs/Makefile > new file mode 100644 > index 000000000000..56b2b48a765b > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/Makefile > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +TEST_GEN_PROGS := dev_in_maps > + > +CFLAGS := -Wall -Werror > + > +include ../../lib.mk > diff --git a/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c > new file mode 100644 > index 000000000000..08497c2e10a3 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/dev_in_maps.c > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#define _GNU_SOURCE > + > +#include <inttypes.h> > +#include <unistd.h> > +#include <stdio.h> > + > +#include <linux/unistd.h> > +#include <linux/types.h> > +#include <linux/mount.h> > +#include <sys/syscall.h> > +#include <sys/stat.h> > +#include <sys/mount.h> > +#include <sys/mman.h> > +#include <sched.h> > +#include <fcntl.h> > + > +#include "../../kselftest.h" > +#include "log.h" > + > +static int sys_fsopen(const char *fsname, unsigned int flags) > +{ > + return syscall(__NR_fsopen, fsname, flags); > +} > + > +static int sys_fsconfig(int fd, unsigned int cmd, const char *key, const char *value, int aux) > +{ > + return syscall(__NR_fsconfig, fd, cmd, key, value, aux); > +} > + > +static int sys_fsmount(int fd, unsigned int flags, unsigned int attr_flags) > +{ > + return syscall(__NR_fsmount, fd, flags, attr_flags); > +} > + > +static int sys_move_mount(int from_dfd, const char *from_pathname, > + int to_dfd, const char *to_pathname, > + unsigned int flags) > +{ > + return syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, to_pathname, flags); > +} > + > +static long get_file_dev_and_inode(void *addr, struct statx *stx) > +{ > + char buf[4096]; > + FILE *mapf; > + > + mapf = fopen("/proc/self/maps", "r"); > + if (mapf == NULL) > + return pr_perror("fopen(/proc/self/maps)"); > + > + while (fgets(buf, sizeof(buf), mapf)) { > + unsigned long start, end; > + uint32_t maj, min; > + __u64 ino; > + > + if (sscanf(buf, "%lx-%lx %*s %*s %x:%x %llx", > + &start, &end, &maj, &min, &ino) != 5) > + return pr_perror("unable to parse: %s", buf); > + if (start == (unsigned long)addr) { > + stx->stx_dev_major = maj; > + stx->stx_dev_minor = min; > + stx->stx_ino = ino; > + return 0; > + } > + } > + > + return pr_err("unable to find the mapping"); > +} > + > +static int ovl_mount(void) > +{ > + int tmpfs, fsfd, ovl; > + > + fsfd = sys_fsopen("tmpfs", 0); > + if (fsfd == -1) > + return pr_perror("fsopen(tmpfs)"); > + > + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) == -1) > + return pr_perror("FSCONFIG_CMD_CREATE"); > + > + tmpfs = sys_fsmount(fsfd, 0, 0); > + if (tmpfs == -1) > + return pr_perror("fsmount"); > + > + close(fsfd); > + > + /* overlayfs can't be constructed on top of a detached mount. */ > + if (sys_move_mount(tmpfs, "", AT_FDCWD, "/tmp", MOVE_MOUNT_F_EMPTY_PATH)) > + return pr_perror("move_mount"); > + close(tmpfs); > + > + if (mkdir("/tmp/w", 0755) == -1 || > + mkdir("/tmp/u", 0755) == -1 || > + mkdir("/tmp/l", 0755) == -1) > + return pr_perror("mkdir"); > + > + fsfd = sys_fsopen("overlay", 0); > + if (fsfd == -1) > + return pr_perror("fsopen(overlay)"); > + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", "test", 0) == -1 || > + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "lowerdir", "/tmp/l", 0) == -1 || > + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "upperdir", "/tmp/u", 0) == -1 || > + sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "workdir", "/tmp/w", 0) == -1) > + return pr_perror("fsconfig"); > + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) == -1) > + return pr_perror("fsconfig"); > + ovl = sys_fsmount(fsfd, 0, 0); > + if (ovl == -1) > + return pr_perror("fsmount"); > + > + return ovl; > +} > + > +/* > + * Check that the file device and inode shown in /proc/pid/maps match values > + * returned by stat(2). > + */ > +static int test(void) > +{ > + struct statx stx, mstx; > + int ovl, fd; > + void *addr; > + > + ovl = ovl_mount(); > + if (ovl == -1) > + return -1; > + > + fd = openat(ovl, "test", O_RDWR | O_CREAT, 0644); > + if (fd == -1) > + return pr_perror("openat"); > + > + addr = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0); > + if (addr == MAP_FAILED) > + return pr_perror("mmap"); > + > + if (get_file_dev_and_inode(addr, &mstx)) > + return -1; > + if (statx(fd, "", AT_EMPTY_PATH | AT_STATX_SYNC_AS_STAT, STATX_INO, &stx)) > + return pr_perror("statx"); > + > + if (stx.stx_dev_major != mstx.stx_dev_major || > + stx.stx_dev_minor != mstx.stx_dev_minor || > + stx.stx_ino != mstx.stx_ino) > + return pr_fail("unmatched dev:ino %x:%x:%llx (expected %x:%x:%llx)\n", > + mstx.stx_dev_major, mstx.stx_dev_minor, mstx.stx_ino, > + stx.stx_dev_major, stx.stx_dev_minor, stx.stx_ino); > + > + ksft_test_result_pass("devices are matched\n"); > + return 0; > +} > + > +int main(int argc, char **argv) > +{ > + int fsfd; > + > + fsfd = sys_fsopen("overlay", 0); > + if (fsfd == -1) { > + ksft_test_result_skip("unable to create overlay mount\n"); > + return 1; > + } > + close(fsfd); > + > + /* Create a new mount namespace to not care about cleaning test mounts. */ > + if (unshare(CLONE_NEWNS) == -1) { > + ksft_test_result_skip("unable to create a new mount namespace\n"); > + return 1; > + } > + > + if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) == -1) { > + pr_perror("mount"); > + return 1; > + } > + > + ksft_set_plan(1); > + > + if (test()) > + return 1; > + > + ksft_exit_pass(); > + return 0; > +} > diff --git a/tools/testing/selftests/filesystems/overlayfs/log.h b/tools/testing/selftests/filesystems/overlayfs/log.h > new file mode 100644 > index 000000000000..db64df2a8483 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/overlayfs/log.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef __SELFTEST_TIMENS_LOG_H__ > +#define __SELFTEST_TIMENS_LOG_H__ > + > +#define pr_msg(fmt, lvl, ...) \ > + ksft_print_msg("[%s] (%s:%d)\t" fmt "\n", \ > + lvl, __FILE__, __LINE__, ##__VA_ARGS__) > + > +#define pr_p(func, fmt, ...) func(fmt ": %m", ##__VA_ARGS__) > + > +#define pr_err(fmt, ...) \ > + ({ \ > + ksft_test_result_error(fmt "\n", ##__VA_ARGS__); \ > + -1; \ > + }) > + > +#define pr_fail(fmt, ...) \ > + ({ \ > + ksft_test_result_fail(fmt, ##__VA_ARGS__); \ > + -1; \ > + }) > + > +#define pr_perror(fmt, ...) pr_p(pr_err, fmt, ##__VA_ARGS__) > + > +#endif > -- > 2.43.0.472.g3155946c3a-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-14 10:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-14 6:44 [PATCH 1/2 v2] fs/proc: show correct device and inode numbers in /proc/pid/maps Andrei Vagin
2023-12-14 6:44 ` [PATCH 2/2] selftests/overlayfs: verify " Andrei Vagin
2023-12-14 8:03 ` Amir Goldstein
2023-12-14 8:11 ` [PATCH 1/2 v2] fs/proc: show correct " Amir Goldstein
2023-12-14 10:55 ` Christian Brauner
[not found] <20231211193048.580691-1-avagin@google.com>
[not found] ` <20231211193048.580691-2-avagin@google.com>
2023-12-12 5:17 ` [PATCH 2/2] selftests/overlayfs: verify " Amir Goldstein
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).