lkml.org 
[lkml]   [2017]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr_restrict to the maximum value
Date


> -----Original Message-----
> From: keescook@google.com [mailto:keescook@google.com] On Behalf Of Kees
> Cook
> Sent: Wednesday, October 4, 2017 9:43 AM
> To: Tobin C. Harding <me@tobin.cc>
> Cc: Greg KH <gregkh@linuxfoundation.org>; Petr Mladek <pmladek@suse.com>;
> Joe Perches <joe@perches.com>; Ian Campbell <ijc@hellion.org.uk>; Sergey
> Senozhatsky <sergey.senozhatsky@gmail.com>; kernel-
> hardening@lists.openwall.com; LKML <linux-kernel@vger.kernel.org>; Catalin
> Marinas <catalin.marinas@arm.com>; Will Deacon <will.deacon@arm.com>;
> Steven Rostedt <rostedt@goodmis.org>; Roberts, William C
> <william.c.roberts@intel.com>; Chris Fries <cfries@google.com>; Dave Weinstein
> <olorin@google.com>; Linus Torvalds <torvalds@linux-foundation.org>
> Subject: Re: [kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr_restrict to
> the maximum value
>
> On Sat, Sep 30, 2017 at 5:06 PM, Tobin C. Harding <me@tobin.cc> wrote:
> > Set the initial value of kptr_restrict to the maximum setting rather
> > than the minimum setting, to ensure that early boot logging is not
> > leaking information.
> >
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> > lib/vsprintf.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 0271223..e009325
> > 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -396,7 +396,7 @@ struct printf_spec { #define FIELD_WIDTH_MAX ((1
> > << 23) - 1) #define PRECISION_MAX ((1 << 15) - 1)
> >
> > -int kptr_restrict __read_mostly;
> > +int kptr_restrict __read_mostly = 4; /* maximum setting */
> >
> > /*
> > * return non-zero if we should cleanse pointers for %p and %pK specifiers.
>
> I share Linus's concern that making this the unconditional default will break some
> people. The intention here (as stated in the
> changelog) is to cover anything leaked during early boot before sysctl settings can
> change this value. That shouldn't break perf (which should happen after sysctl
> settings), but it _may_ break debugging of early boot. I would hope that nothing
> would be needed there, but if we want to make this more configurable, we may
> want to consider a Kconfig for the default. Perhaps:
>
> -int kptr_restrict __read_mostly;
> +int kptr_restrict __read_mostly = CONFIG_KPTR_RESTRICT_DEFAULT;
>
> ...
>
> +config KPTR_RESTRICT_DEFAULT
> + bool "Avoid leaking kernel pointers to userspace"
> + default 0
> + range 0 4
> + help
> + This specifies the initial value of the kptr_restrict sysctl, which
> + controls the level of kernel pointers removed from display
> + to userspace. 0 = off, 1 = %pK not visible to non-root, 2 = %pK
> + not visible for any user, 3 = %p also not visible, 4 = physical and
> + resource addresses also not visible.
>
>
> I'd argue that a default of "1" would be a sensible starting place, but that can be a
> separate patch, IMO.
>

I might be crazy, as often the case, but a command line option might be useful here
so you can boot the same kernel with Kptr < 4 if you really need to get at any information
hidden by a configure time value of 4.

> -Kees
>
> --
> Kees Cook
> Pixel Security
\
 
 \ /
  Last update: 2017-10-04 18:50    [W:0.105 / U:0.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site