[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161007142955.GB14666@pc.thejh.net>
Date: Fri, 7 Oct 2016 16:29:55 +0200
From: Jann Horn <jann@...jh.net>
To: "Roberts, William C" <william.c.roberts@...el.com>
Cc: Kees Cook <keescook@...omium.org>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: RE: [PATCH] printk: introduce kptr_restrict
level 3
On Fri, Oct 07, 2016 at 02:19:43PM +0000, Roberts, William C wrote:
>
>
> > -----Original Message-----
> > From: keescook@...gle.com [mailto:keescook@...gle.com] On Behalf Of Kees
> > Cook
> > Sent: Thursday, October 6, 2016 5:05 PM
> > To: Roberts, William C <william.c.roberts@...el.com>
> > Cc: kernel-hardening@...ts.openwall.com
> > Subject: Re: [PATCH] printk: introduce kptr_restrict level 3
> >
> > On Thu, Oct 6, 2016 at 8:18 AM, Roberts, William C <william.c.roberts@...el.com>
> > wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: keescook@...gle.com [mailto:keescook@...gle.com] On Behalf Of
> > >> Kees Cook
> > >> Sent: Wednesday, October 5, 2016 3:34 PM
> > >> To: Roberts, William C <william.c.roberts@...el.com>
> > >> Cc: kernel-hardening@...ts.openwall.com; Jonathan Corbet
> > >> <corbet@....net>; linux-doc@...r.kernel.org; LKML
> > >> <linux-kernel@...r.kernel.org>; Nick Desaulniers
> > >> <ndesaulniers@...gle.com>; Dave Weinstein <olorin@...gle.com>
> > >> Subject: Re: [PATCH] printk: introduce kptr_restrict level 3
> > >>
> > >> On Wed, Oct 5, 2016 at 11:04 AM, <william.c.roberts@...el.com> wrote:
> > >> > From: William Roberts <william.c.roberts@...el.com>
> > >> >
> > >> > Some out-of-tree modules do not use %pK and just use %p, as it's
> > >> > the common C paradigm for printing pointers. Because of this,
> > >> > kptr_restrict has no affect on the output and thus, no way to
> > >> > contain the kernel address leak.
> > >>
> > >> Solving this is certainly a good idea -- I'm all for finding a solid solution.
> > >>
> > >> > Introduce kptr_restrict level 3 that causes the kernel to treat %p
> > >> > as if it was %pK and thus always prints zeros.
> > >>
> > >> I'm worried that this could break kernel internals where %p is being
> > >> used and not exposed to userspace. Maybe those situations don't exist...
> > >>
> > >> Regardless, I would rather do what Grsecurity has done in this area,
> > >> and whitelist known-safe values instead. For example, they have %pP
> > >> for approved pointers, and %pX for approved
> > >> dereference_function_descriptor() output. Everything else is censored
> > >> if it is a value in kernel memory and destined for a user-space
> > >> memory
> > >> buffer:
> > >>
> > >> if ((unsigned long)ptr > TASK_SIZE && *fmt != 'P' && *fmt !=
> > >> 'X' && *fmt != 'K' && is_usercopy_object(buf)) {
> > >> printk(KERN_ALERT "grsec: kernel infoleak detected!
> > >> Please report this log to spender@...ecurity.net.\n");
> > >> dump_stack();
> > >> ptr = NULL;
> > >> }
> > >>
> > >> The "is_usercopy_object()" test is something we can add, which is
> > >> testing for a new SLAB flag that is used to mark slab caches as
> > >> either used by user-space or not, which is done also through whitelisting.
> > >> (For more details on this, see:
> > >> http://www.openwall.com/lists/kernel-hardening/2016/06/08/10)
> > >>
> > >> Would you have time/interest to add the slab flags and is_usercopy_object()?
> > >> The hardened usercopy part of the slab whitelisting can be separate,
> > >> since it likely needs a different usercopy interface to sanely integrate with
> > upstream.
> > >
> > > A couple of questions off hand:
> > > 1. What about bss statics? I am assuming that when the loader loads up a
> > module
> > > That it's dynamically allocating the .bss section or some equivalent. I would
> > > Also assume the method you describe would catch that, is that correct?
> > >
> > > 2. What about stack variables?
> >
> > It looks like what Grsecurity is doing is saying "if the address is outside of user-
> > space" (" > TASK_SIZE") and it's not whitelisted ('P',
> > 'X') and it's going to land in a user-space buffer ("is_usercopy_object()", censor it.
> > ("K" is already censored -- they're just optimizing to avoid re-checking it
> > needlessly.)
> >
> > So, in this case, all kernel memory, bss and stack included, would be outside the
> > user-space address range. (I am curious, however, how to apply this to an
> > architecture like s390 which has overlapping address ranges... probably the
> > TASK_SIZE test needs to use some other "is in kernel memory" check that
> > compiles down to TASK_SIZE on non-s390, and DTRT on s390, etc.)
> >
>
> Before I go off and attempt this, I just have another dumb question to ask:
>
> If the printk copies it into the kernel ring buffer, at some point, someone comes
> And asks for a copy into a userspace buffer either via dmesg or proc/kmsg interfaces.
IMO that's fine - I don't think pointers in the kernel ring buffer should be restricted.
Instead, access to dmesg / proc/kmsg should be restricted appropriately.
I guess it depends on what the goal here is. Do we really want to stop root from
ever seeing a kernel pointer (in which case OOPS messages wouldn't really work
anymore)? My view is that restricting these interfaces so far that only root can
access them and it's unlikely that root accidentally does so is sufficient.
Download attachment "signature.asc" of type "application/pgp-signature" (820 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.
