linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>
Subject: [git pull] vfs.git pile 11
Date: Thu, 6 Jul 2017 10:12:56 +0100	[thread overview]
Message-ID: <20170706091256.GN10672@ZenIV.linux.org.uk> (raw)

	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.
	* 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...

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 ea93a426af164d346a0b4fe0836143bf32177330:

  iov_iter: saner checks on copyin/copyout (2017-06-29 22:29:36 -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              | 96 ++++++++++++++++++++++++++++++++-------------
 lib/usercopy.c              | 10 ++++-
 5 files changed, 167 insertions(+), 68 deletions(-)

             reply	other threads:[~2017-07-06  9:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-06  9:12 Al Viro [this message]
2017-07-06 19:45 ` [git pull] vfs.git pile 11 Kees Cook
2017-07-06 20:18   ` Al Viro
2017-07-06 20:29     ` Al Viro
2017-07-06 21:20       ` Al Viro
2017-07-07  5:09         ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170706091256.GN10672@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).