On Fri, May 18, 2018 at 8:43 PM Al Viro <[email protected]> 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,
