[1/2,v2] fs/proc: show correct device and inode numbers in /proc/pid/maps
Commit Message
/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(-)
Comments
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
>
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
@@ -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;
@@ -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)
{