Re: [PATCH v2 4/4] vsprintf: Add command line option debug_early_boot

[Tobin C. Harding]

On Wed, May 02, 2018 at 09:57:57PM -0700, Kees Cook wrote:
> On Wed, May 2, 2018 at 3:50 PM, Tobin C. Harding  wrote:
> > Currently printing [hashed] pointers requires either a hw RNG or enough
> > entropy to be available.  Early in the boot sequence these conditions
> > may not be met resulting in a dummy string '(ptrval)' being
> > printed.  This makes debugging the early boot sequence difficult.  We
> > can relax the requirement to use cryptographically secure hashing during
> > debugging.  This enables debugging while keeping development/production
> > kernel behaviour the same.
> >
> > If new command line option debug_early_boot is enabled use
> > cryptographically insecure hashing and hash pointer value immediately.
> >
> > Signed-off-by: Tobin C. Harding 
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt |  8 
> >  lib/vsprintf.c  | 18 ++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index b8d1379aa039..ab619c4ccbf2 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -748,6 +748,14 @@
> >
> > debug   [KNL] Enable kernel debugging (events log level).
> >
> > +   debug_early_boot
> > +   [KNL] Enable debugging early in the boot sequence.  
> > If
> > +   enabled, we use a weak hash instead of siphash to 
> > hash
> > +   pointers.  Use this option if you need to see 
> > pointer
> > +   values during early boot (i.e you are seeing 
> > instances
> > +   of '(___ptrval___)') - cryptographically insecure,
> > +   please do not use on production kernels.
> > +
> > debug_locks_verbose=
> > [KNL] verbose self-tests
> > Format=<0|1>
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 3697a19c2b25..6c139b442267 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, 
> > struct device_node *dn,
> > return widen_string(buf, buf - buf_start, end, spec);
> >  }
> >
> > +/* Make pointers available for printing early in the boot sequence. */
> > +static int debug_early_boot;
> 
> Please make this __ro_after_init too.

Good suggestion.  I forgot, we are supposed to be closing security
wholes not opening them :)

thanks,
Tobin.

Re: [PATCH v2 4/4] vsprintf: Add command line option debug_early_boot

[Tobin C. Harding]

On Wed, May 02, 2018 at 09:57:57PM -0700, Kees Cook wrote:
> On Wed, May 2, 2018 at 3:50 PM, Tobin C. Harding  wrote:
> > Currently printing [hashed] pointers requires either a hw RNG or enough
> > entropy to be available.  Early in the boot sequence these conditions
> > may not be met resulting in a dummy string '(ptrval)' being
> > printed.  This makes debugging the early boot sequence difficult.  We
> > can relax the requirement to use cryptographically secure hashing during
> > debugging.  This enables debugging while keeping development/production
> > kernel behaviour the same.
> >
> > If new command line option debug_early_boot is enabled use
> > cryptographically insecure hashing and hash pointer value immediately.
> >
> > Signed-off-by: Tobin C. Harding 
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt |  8 
> >  lib/vsprintf.c  | 18 ++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index b8d1379aa039..ab619c4ccbf2 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -748,6 +748,14 @@
> >
> > debug   [KNL] Enable kernel debugging (events log level).
> >
> > +   debug_early_boot
> > +   [KNL] Enable debugging early in the boot sequence.  
> > If
> > +   enabled, we use a weak hash instead of siphash to 
> > hash
> > +   pointers.  Use this option if you need to see 
> > pointer
> > +   values during early boot (i.e you are seeing 
> > instances
> > +   of '(___ptrval___)') - cryptographically insecure,
> > +   please do not use on production kernels.
> > +
> > debug_locks_verbose=
> > [KNL] verbose self-tests
> > Format=<0|1>
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 3697a19c2b25..6c139b442267 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, 
> > struct device_node *dn,
> > return widen_string(buf, buf - buf_start, end, spec);
> >  }
> >
> > +/* Make pointers available for printing early in the boot sequence. */
> > +static int debug_early_boot;
> 
> Please make this __ro_after_init too.

Good suggestion.  I forgot, we are supposed to be closing security
wholes not opening them :)

thanks,
Tobin.

Re: [PATCH v2 4/4] vsprintf: Add command line option debug_early_boot

[Kees Cook]

On Wed, May 2, 2018 at 3:50 PM, Tobin C. Harding  wrote:
> Currently printing [hashed] pointers requires either a hw RNG or enough
> entropy to be available.  Early in the boot sequence these conditions
> may not be met resulting in a dummy string '(ptrval)' being
> printed.  This makes debugging the early boot sequence difficult.  We
> can relax the requirement to use cryptographically secure hashing during
> debugging.  This enables debugging while keeping development/production
> kernel behaviour the same.
>
> If new command line option debug_early_boot is enabled use
> cryptographically insecure hashing and hash pointer value immediately.
>
> Signed-off-by: Tobin C. Harding 
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  8 
>  lib/vsprintf.c  | 18 ++
>  2 files changed, 26 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index b8d1379aa039..ab619c4ccbf2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -748,6 +748,14 @@
>
> debug   [KNL] Enable kernel debugging (events log level).
>
> +   debug_early_boot
> +   [KNL] Enable debugging early in the boot sequence.  If
> +   enabled, we use a weak hash instead of siphash to hash
> +   pointers.  Use this option if you need to see pointer
> +   values during early boot (i.e you are seeing instances
> +   of '(___ptrval___)') - cryptographically insecure,
> +   please do not use on production kernels.
> +
> debug_locks_verbose=
> [KNL] verbose self-tests
> Format=<0|1>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3697a19c2b25..6c139b442267 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, struct 
> device_node *dn,
> return widen_string(buf, buf - buf_start, end, spec);
>  }
>
> +/* Make pointers available for printing early in the boot sequence. */
> +static int debug_early_boot;

Please make this __ro_after_init too.

-Kees

> +EXPORT_SYMBOL(debug_early_boot);
> +
> +static int __init debug_early_boot_enable(char *str)
> +{
> +   debug_early_boot = 1;
> +   pr_info("debug_early_boot enabled\n");
> +   return 0;
> +}
> +early_param("debug_early_boot", debug_early_boot_enable);
> +
>  static bool have_filled_random_ptr_key __read_mostly;
>  static siphash_key_t ptr_key __read_mostly;
>
> @@ -1707,6 +1719,12 @@ static char *ptr_to_id(char *buf, char *end, void 
> *ptr, struct printf_spec spec)
> const char *str = sizeof(ptr) == 8 ? "(ptrval)" : "(ptrval)";
> unsigned long hashval;
>
> +   /* When debugging early boot use non-cryptographically secure hash */
> +   if (unlikely(debug_early_boot)) {
> +   hashval = hash_long((unsigned long)ptr, 32);
> +   return pointer_string(buf, end, (const void *)hashval, spec);
> +   }
> +
> if (unlikely(!have_filled_random_ptr_key)) {
> spec.field_width = 2 * sizeof(ptr);
> /* string length must be less than default_width */
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security

Re: [PATCH v2 4/4] vsprintf: Add command line option debug_early_boot

[Kees Cook]

On Wed, May 2, 2018 at 3:50 PM, Tobin C. Harding  wrote:
> Currently printing [hashed] pointers requires either a hw RNG or enough
> entropy to be available.  Early in the boot sequence these conditions
> may not be met resulting in a dummy string '(ptrval)' being
> printed.  This makes debugging the early boot sequence difficult.  We
> can relax the requirement to use cryptographically secure hashing during
> debugging.  This enables debugging while keeping development/production
> kernel behaviour the same.
>
> If new command line option debug_early_boot is enabled use
> cryptographically insecure hashing and hash pointer value immediately.
>
> Signed-off-by: Tobin C. Harding 
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  8 
>  lib/vsprintf.c  | 18 ++
>  2 files changed, 26 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index b8d1379aa039..ab619c4ccbf2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -748,6 +748,14 @@
>
> debug   [KNL] Enable kernel debugging (events log level).
>
> +   debug_early_boot
> +   [KNL] Enable debugging early in the boot sequence.  If
> +   enabled, we use a weak hash instead of siphash to hash
> +   pointers.  Use this option if you need to see pointer
> +   values during early boot (i.e you are seeing instances
> +   of '(___ptrval___)') - cryptographically insecure,
> +   please do not use on production kernels.
> +
> debug_locks_verbose=
> [KNL] verbose self-tests
> Format=<0|1>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3697a19c2b25..6c139b442267 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, struct 
> device_node *dn,
> return widen_string(buf, buf - buf_start, end, spec);
>  }
>
> +/* Make pointers available for printing early in the boot sequence. */
> +static int debug_early_boot;

Please make this __ro_after_init too.

-Kees

> +EXPORT_SYMBOL(debug_early_boot);
> +
> +static int __init debug_early_boot_enable(char *str)
> +{
> +   debug_early_boot = 1;
> +   pr_info("debug_early_boot enabled\n");
> +   return 0;
> +}
> +early_param("debug_early_boot", debug_early_boot_enable);
> +
>  static bool have_filled_random_ptr_key __read_mostly;
>  static siphash_key_t ptr_key __read_mostly;
>
> @@ -1707,6 +1719,12 @@ static char *ptr_to_id(char *buf, char *end, void 
> *ptr, struct printf_spec spec)
> const char *str = sizeof(ptr) == 8 ? "(ptrval)" : "(ptrval)";
> unsigned long hashval;
>
> +   /* When debugging early boot use non-cryptographically secure hash */
> +   if (unlikely(debug_early_boot)) {
> +   hashval = hash_long((unsigned long)ptr, 32);
> +   return pointer_string(buf, end, (const void *)hashval, spec);
> +   }
> +
> if (unlikely(!have_filled_random_ptr_key)) {
> spec.field_width = 2 * sizeof(ptr);
> /* string length must be less than default_width */
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security

[PATCH v2 4/4] vsprintf: Add command line option debug_early_boot

[Tobin C. Harding]

Currently printing [hashed] pointers requires either a hw RNG or enough
entropy to be available.  Early in the boot sequence these conditions
may not be met resulting in a dummy string '(ptrval)' being
printed.  This makes debugging the early boot sequence difficult.  We
can relax the requirement to use cryptographically secure hashing during
debugging.  This enables debugging while keeping development/production
kernel behaviour the same.

If new command line option debug_early_boot is enabled use
cryptographically insecure hashing and hash pointer value immediately.

Signed-off-by: Tobin C. Harding 
---
 Documentation/admin-guide/kernel-parameters.txt |  8 
 lib/vsprintf.c  | 18 ++
 2 files changed, 26 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index b8d1379aa039..ab619c4ccbf2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -748,6 +748,14 @@
 
debug   [KNL] Enable kernel debugging (events log level).
 
+   debug_early_boot
+   [KNL] Enable debugging early in the boot sequence.  If
+   enabled, we use a weak hash instead of siphash to hash
+   pointers.  Use this option if you need to see pointer
+   values during early boot (i.e you are seeing instances
+   of '(___ptrval___)') - cryptographically insecure,
+   please do not use on production kernels.
+
debug_locks_verbose=
[KNL] verbose self-tests
Format=<0|1>
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3697a19c2b25..6c139b442267 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, struct 
device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
 }
 
+/* Make pointers available for printing early in the boot sequence. */
+static int debug_early_boot;
+EXPORT_SYMBOL(debug_early_boot);
+
+static int __init debug_early_boot_enable(char *str)
+{
+   debug_early_boot = 1;
+   pr_info("debug_early_boot enabled\n");
+   return 0;
+}
+early_param("debug_early_boot", debug_early_boot_enable);
+
 static bool have_filled_random_ptr_key __read_mostly;
 static siphash_key_t ptr_key __read_mostly;
 
@@ -1707,6 +1719,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, 
struct printf_spec spec)
const char *str = sizeof(ptr) == 8 ? "(ptrval)" : "(ptrval)";
unsigned long hashval;
 
+   /* When debugging early boot use non-cryptographically secure hash */
+   if (unlikely(debug_early_boot)) {
+   hashval = hash_long((unsigned long)ptr, 32);
+   return pointer_string(buf, end, (const void *)hashval, spec);
+   }
+
if (unlikely(!have_filled_random_ptr_key)) {
spec.field_width = 2 * sizeof(ptr);
/* string length must be less than default_width */
-- 
2.7.4

[PATCH v2 4/4] vsprintf: Add command line option debug_early_boot

[Tobin C. Harding]

Currently printing [hashed] pointers requires either a hw RNG or enough
entropy to be available.  Early in the boot sequence these conditions
may not be met resulting in a dummy string '(ptrval)' being
printed.  This makes debugging the early boot sequence difficult.  We
can relax the requirement to use cryptographically secure hashing during
debugging.  This enables debugging while keeping development/production
kernel behaviour the same.

If new command line option debug_early_boot is enabled use
cryptographically insecure hashing and hash pointer value immediately.

Signed-off-by: Tobin C. Harding 
---
 Documentation/admin-guide/kernel-parameters.txt |  8 
 lib/vsprintf.c  | 18 ++
 2 files changed, 26 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index b8d1379aa039..ab619c4ccbf2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -748,6 +748,14 @@
 
debug   [KNL] Enable kernel debugging (events log level).
 
+   debug_early_boot
+   [KNL] Enable debugging early in the boot sequence.  If
+   enabled, we use a weak hash instead of siphash to hash
+   pointers.  Use this option if you need to see pointer
+   values during early boot (i.e you are seeing instances
+   of '(___ptrval___)') - cryptographically insecure,
+   please do not use on production kernels.
+
debug_locks_verbose=
[KNL] verbose self-tests
Format=<0|1>
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3697a19c2b25..6c139b442267 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, struct 
device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
 }
 
+/* Make pointers available for printing early in the boot sequence. */
+static int debug_early_boot;
+EXPORT_SYMBOL(debug_early_boot);
+
+static int __init debug_early_boot_enable(char *str)
+{
+   debug_early_boot = 1;
+   pr_info("debug_early_boot enabled\n");
+   return 0;
+}
+early_param("debug_early_boot", debug_early_boot_enable);
+
 static bool have_filled_random_ptr_key __read_mostly;
 static siphash_key_t ptr_key __read_mostly;
 
@@ -1707,6 +1719,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, 
struct printf_spec spec)
const char *str = sizeof(ptr) == 8 ? "(ptrval)" : "(ptrval)";
unsigned long hashval;
 
+   /* When debugging early boot use non-cryptographically secure hash */
+   if (unlikely(debug_early_boot)) {
+   hashval = hash_long((unsigned long)ptr, 32);
+   return pointer_string(buf, end, (const void *)hashval, spec);
+   }
+
if (unlikely(!have_filled_random_ptr_key)) {
spec.field_width = 2 * sizeof(ptr);
/* string length must be less than default_width */
-- 
2.7.4