[kernel-hardening] Re: [PATCH v2 1/4] init: create cmdline param to disable readonly

[Ingo Molnar <mingo@kernel.org>]

* Kees Cook <keescook@chromium.org> wrote:

> It may be useful to debug writes to the readonly sections of memory,
> so provide a cmdline "rodata=off" to allow for this.
> 
> Suggested-by: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  Documentation/kernel-parameters.txt |  4 ++++
>  init/main.c                         | 31 +++++++++++++++++++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 742f69d18fc8..21cf76dbba90 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3409,6 +3409,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  
>  	ro		[KNL] Mount root device read-only on boot
>  
> +	rodata=		[KNL]
> +		on	Mark read-only kernel memory as read-only (default).
> +		off	Leave read-only kernel memory writable for debugging.
> +
>  	root=		[KNL] Root filesystem
>  			See name_to_dev_t comment in init/do_mounts.c.
>  
> diff --git a/init/main.c b/init/main.c
> index 9e64d7097f1a..06200d2fbf08 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -93,9 +93,6 @@ static int kernel_init(void *);
>  extern void init_IRQ(void);
>  extern void fork_init(void);
>  extern void radix_tree_init(void);
> -#ifndef CONFIG_DEBUG_RODATA
> -static inline void mark_rodata_ro(void) { }
> -#endif
>  
>  /*
>   * Debug helper: via this flag we know that we are in 'early bootup code'
> @@ -929,6 +926,32 @@ static int try_to_run_init_process(const char *init_filename)
>  
>  static noinline void __init kernel_init_freeable(void);
>  
> +#ifdef CONFIG_DEBUG_RODATA

Btw., could you please remove the Kconfig option altogether in an additional patch 
and make read-only sections an always-on feature? It has been default-y for years 
and all distros have it enabled.

The 'debug rodata' naming is purely historic: this started out as a simple 
debugging feature, but meanwhile it has spread and has become an essential kernel 
robustness feature.

The boot option you added can be used if anyone needs to disable it. (Never heard 
of such a case though.)

Thanks,

	Ingo