Messages in this thread Patch in this message | | | From | Linus Torvalds <> | Date | Fri, 18 May 2018 21:12:56 -0700 | Subject | Re: [PATCH] procfs: fix mmap() for /proc/vmcore |
| |
On Fri, May 18, 2018 at 8:43 PM Al Viro <viro@zeniv.linux.org.uk> 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, | |