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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 May 2018 21:12:56 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     gor@...ux.ibm.com, Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Heiko Carstens <heiko.carstens@...ibm.com>
Subject: Re: [PATCH] procfs: fix mmap() for /proc/vmcore

On Fri, May 18, 2018 at 8:43 PM Al Viro <viro@...iv.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

View attachment "patch.diff" of type "text/x-patch" (658 bytes)