Re: [kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr_restrict to the maximum value

[Kees Cook]

On Sat, Sep 30, 2017 at 5:06 PM, Tobin C. Harding <me@xxxxxxxx> 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@xxxxxxxx>
> ---
>  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