[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jJQOduZ5evB=1w5NymUFbDNoeB4Y4u2a7zJ8k8vsAojRQ@mail.gmail.com>
Date: Wed, 4 Oct 2017 09:42:51 -0700
From: Kees Cook <keescook@...omium.org>
To: "Tobin C. Harding" <me@...in.cc>
Cc: Greg KH <gregkh@...uxfoundation.org>, Petr Mladek <pmladek@...e.com>,
Joe Perches <joe@...ches.com>, Ian Campbell <ijc@...lion.org.uk>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, LKML <linux-kernel@...r.kernel.org>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will.deacon@....com>,
Steven Rostedt <rostedt@...dmis.org>, William Roberts <william.c.roberts@...el.com>,
Chris Fries <cfries@...gle.com>, Dave Weinstein <olorin@...gle.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [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@...in.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@...in.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.
-Kees
--
Kees Cook
Pixel Security
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.
