diff mbox series

[v2,2/3] fs: create helper file_user_path() for user displayed mapped file path

Message ID 20231007084433.1417887-3-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series Reduce impact of overlayfs backing files fake path | expand

Commit Message

Amir Goldstein Oct. 7, 2023, 8:44 a.m. UTC 
Overlayfs uses backing files with "fake" overlayfs f_path and "real"
underlying f_inode, in order to use underlying inode aops for mapped
files and to display the overlayfs path in /proc/<pid>/maps.

In preparation for storing the overlayfs "fake" path instead of the
underlying "real" path in struct backing_file, define a noop helper
file_user_path() that returns f_path for now.

Use the new helper in procfs and kernel logs whenever a path of a
mapped file is displayed to users.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 arch/arc/kernel/troubleshoot.c |  3 ++-
 fs/proc/base.c                 |  2 +-
 fs/proc/nommu.c                |  2 +-
 fs/proc/task_mmu.c             |  4 ++--
 fs/proc/task_nommu.c           |  2 +-
 include/linux/fs.h             | 14 ++++++++++++++
 kernel/trace/trace_output.c    |  2 +-
 7 files changed, 22 insertions(+), 7 deletions(-)

Comments

Al Viro Oct. 9, 2023, 7 a.m. UTC | #1 
On Sat, Oct 07, 2023 at 11:44:32AM +0300, Amir Goldstein wrote:

> @@ -93,7 +93,8 @@ static void show_faulting_vma(unsigned long address)
>  		char *nm = "?";
>  
>  		if (vma->vm_file) {
> -			nm = file_path(vma->vm_file, buf, ARC_PATH_MAX-1);
> +			nm = d_path(file_user_path(vma->vm_file), buf,
> +				    ARC_PATH_MAX-1);
>  			if (IS_ERR(nm))
>  				nm = "?";

Umm...  At one point I considered this:
	if (vma->vm_file)
                pr_info("  @off 0x%lx in [%pD]  VMA: 0x%08lx to 0x%08lx\n",
                        vma->vm_start < TASK_UNMAPPED_BASE ?
                                address : address - vma->vm_start,
                        vma->vm_file, vma->vm_start, vma->vm_end);
	else
                pr_info("  @off 0x%lx in [anon]  VMA: 0x%08lx to 0x%08lx\n",
                        vma->vm_start < TASK_UNMAPPED_BASE ?
                                address : address - vma->vm_start,
                        vma->vm_start, vma->vm_end);
and to hell with that 'buf' thing...
Amir Goldstein Oct. 9, 2023, 7:51 a.m. UTC | #2 
On Mon, Oct 9, 2023 at 10:00 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, Oct 07, 2023 at 11:44:32AM +0300, Amir Goldstein wrote:
>
> > @@ -93,7 +93,8 @@ static void show_faulting_vma(unsigned long address)
> >               char *nm = "?";
> >
> >               if (vma->vm_file) {
> > -                     nm = file_path(vma->vm_file, buf, ARC_PATH_MAX-1);
> > +                     nm = d_path(file_user_path(vma->vm_file), buf,
> > +                                 ARC_PATH_MAX-1);
> >                       if (IS_ERR(nm))
> >                               nm = "?";
>
> Umm...  At one point I considered this:
>         if (vma->vm_file)
>                 pr_info("  @off 0x%lx in [%pD]  VMA: 0x%08lx to 0x%08lx\n",
>                         vma->vm_start < TASK_UNMAPPED_BASE ?
>                                 address : address - vma->vm_start,
>                         vma->vm_file, vma->vm_start, vma->vm_end);
>         else
>                 pr_info("  @off 0x%lx in [anon]  VMA: 0x%08lx to 0x%08lx\n",
>                         vma->vm_start < TASK_UNMAPPED_BASE ?
>                                 address : address - vma->vm_start,
>                         vma->vm_start, vma->vm_end);
> and to hell with that 'buf' thing...

It's fine by me.
That would be consistent with print_bad_pte().
I've never had to debug vma/pte faults, so I don't know
how much of the path is really valuable for debugging.

Thanks,
Amir.
diff mbox series

Patch

diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
index d5b3ed2c58f5..097887e4a9b7 100644
--- a/arch/arc/kernel/troubleshoot.c
+++ b/arch/arc/kernel/troubleshoot.c
@@ -93,7 +93,8 @@  static void show_faulting_vma(unsigned long address)
 		char *nm = "?";
 
 		if (vma->vm_file) {
-			nm = file_path(vma->vm_file, buf, ARC_PATH_MAX-1);
+			nm = d_path(file_user_path(vma->vm_file), buf,
+				    ARC_PATH_MAX-1);
 			if (IS_ERR(nm))
 				nm = "?";
 		}
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ffd54617c354..20695c928ee6 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2218,7 +2218,7 @@  static int map_files_get_link(struct dentry *dentry, struct path *path)
 	rc = -ENOENT;
 	vma = find_exact_vma(mm, vm_start, vm_end);
 	if (vma && vma->vm_file) {
-		*path = vma->vm_file->f_path;
+		*path = *file_user_path(vma->vm_file);
 		path_get(path);
 		rc = 0;
 	}
diff --git a/fs/proc/nommu.c b/fs/proc/nommu.c
index 4d3493579458..c6e7ebc63756 100644
--- a/fs/proc/nommu.c
+++ b/fs/proc/nommu.c
@@ -58,7 +58,7 @@  static int nommu_region_show(struct seq_file *m, struct vm_region *region)
 
 	if (file) {
 		seq_pad(m, ' ');
-		seq_file_path(m, file, "");
+		seq_path(m, file_user_path(file), "");
 	}
 
 	seq_putc(m, '\n');
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 3dd5be96691b..1593940ca01e 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -296,7 +296,7 @@  show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 		if (anon_name)
 			seq_printf(m, "[anon_shmem:%s]", anon_name->name);
 		else
-			seq_file_path(m, file, "\n");
+			seq_path(m, file_user_path(file), "\n");
 		goto done;
 	}
 
@@ -1967,7 +1967,7 @@  static int show_numa_map(struct seq_file *m, void *v)
 
 	if (file) {
 		seq_puts(m, " file=");
-		seq_file_path(m, file, "\n\t= ");
+		seq_path(m, file_user_path(file), "\n\t= ");
 	} else if (vma_is_initial_heap(vma)) {
 		seq_puts(m, " heap");
 	} else if (vma_is_initial_stack(vma)) {
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 7cebd397cc26..bce674533000 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -157,7 +157,7 @@  static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
 
 	if (file) {
 		seq_pad(m, ' ');
-		seq_file_path(m, file, "");
+		seq_path(m, file_user_path(file), "");
 	} else if (mm && vma_is_initial_stack(vma)) {
 		seq_pad(m, ' ');
 		seq_puts(m, "[stack]");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7f7e7d2efbeb..a8e4e1cac48e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2472,6 +2472,20 @@  static inline const struct path *file_real_path(struct file *f)
 	return &f->f_path;
 }
 
+/*
+ * 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.
+ */
+static inline const struct path *file_user_path(struct file *f)
+{
+	return &f->f_path;
+}
+
 static inline struct file *file_clone_open(struct file *file)
 {
 	return dentry_open(&file->f_path, file->f_flags, file->f_cred);
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index db575094c498..d8b302d01083 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -404,7 +404,7 @@  static int seq_print_user_ip(struct trace_seq *s, struct mm_struct *mm,
 			vmstart = vma->vm_start;
 		}
 		if (file) {
-			ret = trace_seq_path(s, &file->f_path);
+			ret = trace_seq_path(s, file_user_path(file));
 			if (ret)
 				trace_seq_printf(s, "[+0x%lx]",
 						 ip - vmstart);