* [git pull] vfs.git pile 11 - iov_iter/hardening
@ 2017-07-08 0:29 Al Viro
2017-07-08 4:36 ` Linus Torvalds
0 siblings, 1 reply; 2+ messages in thread
From: Al Viro @ 2017-07-08 0:29 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
iov_iter/uaccess/hardening pile. For one thing, it trims the
inline part of copy_to_user/copy_from_user to the minimum that *does*
need to be inlined - object size checks, basically. For another,
it sanitizes the checks for iov_iter primitives. There are 4 groups
of checks: access_ok(), might_fault(), object size and KASAN.
* access_ok() had been verified by whoever had set the iov_iter
up. However, that has happened in a function far away, so proving that
there's no path to actual copying bypassing those checks is hard and
proving that iov_iter has not been buggered in the meanwhile is also
not pleasant. So we want those redone in actual copyin/copyout.
* might_fault() is better off consolidated - we know whether
it needs to be checked as soon as we enter iov_iter primitive and
observe the iov_iter flavour. No need to wait until the copyin/copyout.
The call chains are short enough to make sure we won't miss anything -
in fact, it's more robust that way, since there are cases where we do
e.g. forced fault-in before getting to copyin/copyout. It's not quite
what we need to check (in particular, combination of iovec-backed
and set_fs(KERNEL_DS) is almost certainly a bug, not a cause to skip
checks), but that's for later series. For now let's keep might_fault().
* KASAN checks belong in copyin/copyout - at the same level
where other iov_iter flavours would've hit them in memcpy().
* object size checks should apply to *all* iov_iter flavours,
not just iovec-backed ones.
There are two groups of primitives - one gets the kernel object
described as pointer + size (copy_to_iter(), etc.) while another gets
it as page + offset + size (copy_page_to_iter(), etc.)
For the first group the checks are best done where we actually
have a chance to find the object size. In other words, those belong in
inline wrappers in uio.h, before calling into iov_iter.c. Same kind
as we have for inlined part of copy_to_user().
For the second group there is no object to look at - offset in
page is just a number, it bears no type information. So we do them
in the common helper called by iov_iter.c primitives of that kind.
All it currently does is checking that we are not trying to access
outside of the compound page; eventually we might want to add some
sanity checks on the page involved.
So the things we need in copyin/copyout part of iov_iter.c
do not quite match anything in uaccess.h (we want no zeroing, we *do*
want access_ok() and KASAN and we want no might_fault() or object size
checks done on that level). OTOH, these needs are simple enough to
provide a couple of helpers (static in iov_iter.c) doing just what
we need...
Trivial conflicts with libnvdimm; this stuff will get some
followups, but again, that's for another series.
The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:
Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git uaccess-work.iov_iter
for you to fetch changes up to 09fc68dc66f7597bdc8898c991609a48f061bed5:
iov_iter: saner checks on copyin/copyout (2017-07-07 05:18:09 -0400)
----------------------------------------------------------------
Al Viro (5):
copy_{from,to}_user(): move kasan checks and might_fault() out-of-line
copy_{to,from}_user(): consolidate object size checks
iov_iter/hardening: move object size checks to inlined part
iov_iter: sanity checks for copy to/from page primitives
iov_iter: saner checks on copyin/copyout
include/linux/thread_info.h | 27 +++++++++++++
include/linux/uaccess.h | 44 +++++----------------
include/linux/uio.h | 58 +++++++++++++++++++++++++---
lib/iov_iter.c | 94 ++++++++++++++++++++++++++++++++-------------
lib/usercopy.c | 10 ++++-
5 files changed, 165 insertions(+), 68 deletions(-)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [git pull] vfs.git pile 11 - iov_iter/hardening
2017-07-08 0:29 [git pull] vfs.git pile 11 - iov_iter/hardening Al Viro
@ 2017-07-08 4:36 ` Linus Torvalds
0 siblings, 0 replies; 2+ messages in thread
From: Linus Torvalds @ 2017-07-08 4:36 UTC (permalink / raw)
To: Al Viro, Dan Williams
Cc: Linux Kernel Mailing List, linux-fsdevel, Oleg Drokin,
Andreas Dilger, James Simmons
On Fri, Jul 7, 2017 at 5:29 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Trivial conflicts with libnvdimm; this stuff will get some
> followups, but again, that's for another series.
Gaah. Yeah, I guess I could have done the trivial ugly merge that just
took the new copy_from_iter_flushcache() as-is, and didn't match it up
with all the new iov_iter hardening.
Except I decided I don't want that, and want to do a proper merge instead.
Which made the trivial merge something that actually changed that
copy_from_iter_flushcache() logic, and maybe I screwed it up in the
process.
It builds, and it looks right to me, but you and Dan should really
check out the end result.
Particularly so for the CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE logic,
which is where this differs from the other cases, and which I changed
to make the #ifdef less noticeable.
I did try both a i386 and a x86-64 build of the iov_iter code, since
that should test both of those ARCH_HAS cases, but that was purely a
build test.
Also, the new __must_check warnings do trigger. At least for lustre. I
couldn't be arsed to try to fix those, since .. lustre. But I'm also
adding a couple of lustre people to the cc, just to make them aware of
it.. I'd really like the allmodconfig build to be clean.
Linus
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-07-08 4:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-08 0:29 [git pull] vfs.git pile 11 - iov_iter/hardening Al Viro
2017-07-08 4:36 ` Linus Torvalds
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox