From: David Miller <davem@redhat.com>
To: james.hogan@imgtec.com
Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
keescook@chromium.org, akpm@linux-foundation.org,
monstr@monstr.eu, lftan@altera.com, jonas@southpole.se,
chris@zankel.net, jcmvbkbc@gmail.com,
nios2-dev@lists.rocketboards.org, linux@lists.openrisc.net,
linux-xtensa@linux-xtensa.org
Subject: Re: [PATCH v2 00/11] test_user_copy improvements
Date: Mon, 10 Aug 2015 15:29:38 -0700 (PDT) [thread overview]
Message-ID: <20150810.152938.1076489414700359615.davem@redhat.com> (raw)
In-Reply-To: <1438960924-23628-1-git-send-email-james.hogan@imgtec.com>
From: James Hogan <james.hogan@imgtec.com>
Date: Fri, 7 Aug 2015 16:21:53 +0100
> These patches extend the test_user_copy test module to handle lots more
> cases of user accessors which architectures can override separately, and
> in particular those which are important for checking the MIPS Enhanced
> Virtual Addressing (EVA) implementations, which need to handle
> overlapping user and kernel address spaces, with special instructions
> for accessing user address space from kernel mode.
>
> - Checking that kernel pointers are accepted when user address limit is
> set to KERNEL_DS, as done by the kernel when it internally invokes
> system calls with kernel pointers.
> - Checking of the unchecked accessors (which don't call access_ok()).
> Some of the tests are special cased for EVA at the moment which has
> stricter hardware guarantees for bad user accesses than other
> configurations.
> - Checking of other sets of user accessors, including the inatomic user
> copies, clear_user, compatibility accessors (copy_in_user and
> _unaligned), the user string accessors, and the user checksum
> functions, all of which need special handling in arch code with EVA.
>
> Tested on MIPS with and without EVA, and on x86_64.
>
> Only build tested for arm, blackfin, metag, microblaze, openrisc,
> parisc, powerpc, sh, sparc, tile, i386 & xtensa.
>
> All arches were audited for the appropriate exports, only score is known
> to still be missing some.
James, thanks for doing this work.
If I understand the MIPS EVA facility correctly, it operates exactly like
how sparc64 does. Wherein user and kernel virtual addresses are fully
segregated, and one must use a specially tagged load or store to access
user addresses.
This actually creates problems for the tests as currently coded on
such systems (this problem existed before your changes). You might
not be triggering this problem on MIPS EV but it certainly is there.
For example, consider this test:
ret |= test(!copy_from_user(bad_usermem, (char __user *)kmem,
PAGE_SIZE),
"illegal reversed copy_from_user passed");
If the 'kmem' access faults, we will try to zero out PAGE_SIZE bytes
at 'bad_usermem'. But this is not necessarily going to fail.
The user address 'bad_usermem', on MIPS EV and sparc64, could just as
equally happen to be a legitimate kernel address. So this clear will
succeed and we will end up clearing memory at an arbitrary kernel
address.
There is no real way to trap this situation as a native load/store
will work just fine on these addresses.
I don't have a good suggestion other than to say that these tests
seem to only be valid in a combined kernel/user address space, ie.
for systems other than MIPS EV and sparc64.
Also, I think the tests you added and protected with MIPS ifdefs could
equally be enabled on sparc64.
Thanks!
next prev parent reply other threads:[~2015-08-10 22:29 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-07 15:21 [PATCH v2 00/11] test_user_copy improvements James Hogan
2015-08-07 15:21 ` [PATCH v2 01/11] microblaze: Export __strnlen_user to modules James Hogan
2015-08-07 15:21 ` James Hogan
2015-08-07 15:21 ` [PATCH v2 02/11] nios2: Export strncpy_from_user / strnlen_user " James Hogan
2015-08-07 15:21 ` James Hogan
2015-08-10 8:10 ` Ley Foon Tan
2015-08-07 15:21 ` [PATCH v2 03/11] openrisc: Export __clear_user " James Hogan
2015-08-07 15:21 ` James Hogan
2015-08-07 15:21 ` [PATCH v2 04/11] xtensa: Export __strnlen_user " James Hogan
2015-08-07 15:21 ` James Hogan
2015-08-07 15:21 ` [PATCH v2 05/11] test_user_copy: Check legit kernel accesses James Hogan
2015-08-07 15:21 ` James Hogan
2015-08-07 15:21 ` [PATCH v2 06/11] test_user_copy: Check unchecked accessors James Hogan
2015-08-07 15:21 ` James Hogan
2015-08-07 15:22 ` [PATCH v2 07/11] test_user_copy: Check __copy_{to,from}_user_inatomic() James Hogan
2015-08-07 15:22 ` [PATCH v2 08/11] test_user_copy: Check __clear_user()/clear_user() James Hogan
2015-08-07 15:22 ` James Hogan
2015-08-07 15:22 ` [PATCH v2 09/11] test_user_copy: Check user string accessors James Hogan
2015-08-07 15:22 ` James Hogan
2015-08-07 15:22 ` [PATCH v2 10/11] test_user_copy: Check user compatibility accessors James Hogan
2015-08-07 15:22 ` James Hogan
2015-08-07 15:22 ` [PATCH v2 11/11] test_user_copy: Check user checksum functions James Hogan
2015-08-07 15:22 ` James Hogan
2015-08-07 23:51 ` [PATCH v2 00/11] test_user_copy improvements Kees Cook
2015-08-10 22:29 ` David Miller [this message]
2015-08-11 4:08 ` David Miller
2015-08-11 4:08 ` David Miller
2015-08-11 11:20 ` Geert Uytterhoeven
2015-08-12 21:34 ` David Miller
2015-08-11 11:07 ` James Hogan
2015-08-11 17:32 ` David Miller
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=20150810.152938.1076489414700359615.davem@redhat.com \
--to=davem@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=chris@zankel.net \
--cc=james.hogan@imgtec.com \
--cc=jcmvbkbc@gmail.com \
--cc=jonas@southpole.se \
--cc=keescook@chromium.org \
--cc=lftan@altera.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xtensa@linux-xtensa.org \
--cc=linux@lists.openrisc.net \
--cc=monstr@monstr.eu \
--cc=nios2-dev@lists.rocketboards.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).