Re: [PATCH] procfs: fix mmap() for /proc/vmcore

[Linus Torvalds]

On Fri, May 18, 2018 at 8:43 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:

> Not quite.  The things like
>          if (unlikely(*ppos >= inode->i_sb->s_maxbytes))
>                  return 0;
>          iov_iter_truncate(iter, inode->i_sb->s_maxbytes);
> protect most of the regular files (see mm/filemap.c).  And for devices
(which is
> where the majority of crap ->read()/->write() is) it's obviously not
applicable -
> ->s_maxbytes of *what*?

Yeah that "s_maxbytes of what" is I think the real issue. We should never
have made s_maxbytes be super-block specific: we should have made it be
per-inode, and then have inode_init_always() initialize it using something
like the file_mmap_size_max() logic.

(So we'd still have a "sb_maxbytes" that filesystems would fill in, but it
would only be used as a "fill in inode value for regular files for this
superblock").

Then we could actually protect read/write properly, because many of the
nasty bugs have been in character device drivers.

Oh well. It would still be a good thing to do some day, I suspect, but it's
clearly not the case now, and so s_maxbytes actually has much less coverage
than I was hoping for.

(And thus also the problems with /proc/vmcore - it never saw s_maxbytes
limits before).

Oh, well. The lack of any meaningful s_maxbytes coverage for proc obviously
means that my objections against Vasily's patch are mostly invalid. Even if
/proc does use "generic_file_llseek()" a lot and that should limit things
to 4G offsets, you can just use pread64/pwrite64 to see if you can screw up
the offset.

I'd still prefer to limit the damage to just "vmcore".

Something like the below COMPLETELY UNTESTED patch? Vasily?

             Linus
 fs/proc/vmcore.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index a45f0af22a60..83278c547127 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -491,7 +491,15 @@ static int mmap_vmcore(struct file *file, struct vm_area_struct *vma)
 }
 #endif
 
+/* Mark vmcore as being able and willing to do 64-bit mmaps */
+static int vmcore_open(struct inode *inode, struct file *file)
+{
+	file->f_mode |= FMODE_UNSIGNED_OFFSET;
+	return 0;
+}
+
 static const struct file_operations proc_vmcore_operations = {
+	.open		= vmcore_open,
 	.read		= read_vmcore,
 	.llseek		= default_llseek,
 	.mmap		= mmap_vmcore,