Re: [PATCH 2/2] lib/vsprintf: reduce space taken by no_hash_pointers warning

[Petr Mladek]

On Fri 2021-03-05 20:42:06, Marco Elver wrote:
> Move the no_hash_pointers warning string into __initconst section, so
> that it is discarded after init. Remove common start/end characters.
> Also remove repeated lines from the array, since the compiler can't
> remove duplicate strings for us since the array must appear in
> __initconst as defined.
> 
> Note, a similar message appears in kernel/trace/trace.c, but compiling
> the feature is guarded by CONFIG_TRACING. It is not immediately obvious
> if a space-concious kernel would prefer CONFIG_TRACING=n. Therefore, it
> makes sense to keep the message for no_hash_pointers as __initconst, and
> not move the NOTICE-printing to a common function.
> 
> Link: https://lkml.kernel.org/r/CAMuHMdULKZCJevVJcp7TxzLdWLjsQPhE8hqxhnztNi9bjT_cEw@xxxxxxxxxxxxxx
> Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> ---
>  lib/vsprintf.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 4a14889ccb35..1095689c9c97 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2094,26 +2094,30 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
>  bool no_hash_pointers __ro_after_init;
>  EXPORT_SYMBOL_GPL(no_hash_pointers);
>  
> +static const char no_hash_pointers_warning[8][55] __initconst = {
> +	"******************************************************",
> +	"   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   ",
> +	" This system shows unhashed kernel memory addresses   ",
> +	" via the console, logs, and other interfaces. This    ",
> +	" might reduce the security of your system.            ",
> +	" If you see this message and you are not debugging    ",
> +	" the kernel, report this immediately to your system   ",
> +	" administrator!                                       ",
> +};
> +
>  static int __init no_hash_pointers_enable(char *str)
>  {
> +	/* Indices into no_hash_pointers_warning; -1 is an empty line. */
> +	const int lines[] = { 0, 1, -1, 2, 3, 4, -1, 5, 6, 7, -1, 1, 0 };
> +	int i;
> +
>  	if (no_hash_pointers)
>  		return 0;
>  
>  	no_hash_pointers = true;
>  
> -	pr_warn("**********************************************************\n");
> -	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
> -	pr_warn("**                                                      **\n");
> -	pr_warn("** This system shows unhashed kernel memory addresses   **\n");
> -	pr_warn("** via the console, logs, and other interfaces. This    **\n");
> -	pr_warn("** might reduce the security of your system.            **\n");
> -	pr_warn("**                                                      **\n");
> -	pr_warn("** If you see this message and you are not debugging    **\n");
> -	pr_warn("** the kernel, report this immediately to your system   **\n");
> -	pr_warn("** administrator!                                       **\n");
> -	pr_warn("**                                                      **\n");
> -	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
> -	pr_warn("**********************************************************\n");
> +	for (i = 0; i < ARRAY_SIZE(lines); i++)
> +		pr_warn("**%54s**\n", i == -1 ? "" : no_hash_pointers_warning[lines[i]]);

Is this worth it, please? Could anyone provide some numbers how
the kernel size increases between releases?

The number of code lines is basically just growing. The same is true
for the amount of printed messages.

This patch is saving some lines of text that might be effectively
compressed. But it adds some code and array with indexes. Does it
make any significant imrovement in the compressed kernel image?

Geert was primary concerned about the runtime memory consuption.
It will be solved by the  __initconst. The rest affects only
the size of the compressed image on disk.

Best Regards,
Petr