[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 10 Jun 2016 21:08:34 -0400
From: Rik van Riel <riel@...hat.com>
To: kernel-hardening@...ts.openwall.com
Cc: Kees Cook <keescook@...omium.org>, Brad Spengler
<spender@...ecurity.net>,
PaX Team <pageexec@...email.hu>,
Casey Schaufler
<casey.schaufler@...el.com>,
Christoph Lameter <cl@...ux.com>, Pekka Enberg
<penberg@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Joonsoo Kim
<iamjoonsoo.kim@....com>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: Re: [PATCH v2 2/4] usercopy: avoid direct
copying to userspace
On Fri, 2016-06-10 at 14:09 -0700, Kees Cook wrote:
> On Wed, Jun 8, 2016 at 2:11 PM, Kees Cook <keescook@...omium.org>
> wrote:
> >
> > Some non-whitelisted heap memory has small areas that need to be
> > copied
> > to userspace. For these cases, explicitly copy the needed contents
> > out
> > to stack first before sending to userspace. This lets their
> > respective
> > caches remain un-whitelisted (i.e. no SLAB_USERCOPY), since the
> > bulk of
> > their contents should not be exposed to userspace.
> I've spent some time thinking about these kinds of
> non-whitelisted-slab-workaround changes, and I would like to see if
> we
> can design a better solution. So, to that end, here's what I see:
>
> - HARDENED_USERCOPY verifies object addresses and sizes
> - whitelisted caches (via HARDENED_USERCOPY_WHITELIST's
> SLAB_USERCOPY)
> are intentionally rare
> - Some code uses small parts of non-whitelisted cache memory for
> userspace work (I think the auxv ("mm_struct") and signal frames
> ("task_struct") are good examples of this: neither cache should be
> entirely exposed to userspace, yet tiny pieces are sent to
> userspace.)
> - non-whitelist-workarounds are open-coded
> - non-whitelist-workarounds require a double-copy
> - non-whitelist-workarounds have explicit size maximums (e.g.
> AT_VECTOR_SIZE, sizeof(sigset_t))
> - non-whitelist-workarounds _bypass_ HARDENED_USERCOPY object address
> checking
>
> So, while the workarounds do have a max-size sanity-check, they
> actually lack the object address checking that would normally happen
> with the usercopy checks. I think to solve the open-coding and
> double-copy problems without compromising on the whitelisting or the
> explicit size checking, we could also gain back the address checking
> if we created something like:
>
> copy_to_user_n(user, kernel, dynamic-size, const-max-size);
>
> If "const-max-size" isn't detected as a builtin_constant it could
> fail
> to build. When run, it would a) verify dynamic-size wasn't larger
> that
> const-max-size, and b) perform the regular usercopy checks (without
> the SLAB_USERCOPY check).
>
> So, for the auxv example, instead of the new stack variable, the
> memcpy, etc, it could just be a one-line change replacing the
> existing
> copy_to_user() call:
>
> copy_to_user_n(sp, elf_info, ei_index * sizeof(elf_addr_t),
> AT_VECTOR_SIZE);
>
> (Bike-shedding: copy_to_user_bounded(), ..._limited(),
> ..._whitelist_hole(), ?)
>
> What do people think?
I like your idea a lot.
For some kinds of objects, we could go one further.
Eg. for objects we know to be in the slab, we could use
copy_to_user_slab, and fail the copy if the pointer is
not a slab object.
--
All Rights Reversed.
Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
