Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue 2021-03-02 14:49:42, Geert Uytterhoeven wrote:
> Hi Vlastimil, Petr,
>
> On Tue, Mar 2, 2021 at 2:37 PM Vlastimil Babka wrote:
> > On 3/2/21 2:29 PM, Petr Mladek wrote:
> > > On Tue 2021-03-02 13:51:35, Geert Uytterhoeven wrote:
> > >> > > > +
> > >> > > > +
> > >> > > > 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");
> > >> > > > +
> > >> > > > + return 0;
> > >> > > > +}
> > >> > > > +early_param("no_hash_pointers", no_hash_pointers_enable);
> > >> > >
> > >> > > While bloat-o-meter is not smart enough to notice the real size
> > >> > > impact,
> > >> > > this does add more than 500 bytes of string data to the kernel.
> > >> > > Do we really need such a large message?
> > >> > > Perhaps the whole no_hash_pointers machinery should be protected by
> > >> > > "#ifdef CONFIG_DEBUG_KERNEL"?
> >
> > I think it's a no-go only when enabling such option equals to
> > "no_hash_pointers"
> > being always passed. What Geert suggests is that you need both
> > CONFIG_DEBUG_KERNEL *and* no_hash_pointers and that's different.
>
> Exactly.
>
> > So this is basically a kernel tinyfication issue, right? Is that still
> > pursued
> > today? Are there better config options suitable for this than
> > CONFIG_DEBUG_KERNEL?
>
> As long as I hear about products running Linux on SoCs with 10 MiB of
> SRAM, I think the answer is yes.
> I'm not immediately aware of a better config option. There are no more
> TINY options left, and EXPERT selects DEBUG_KERNEL.
DEBUG_KERNEL might actually makes sense. A possibility to see real
pointers might be pretty useful for the other debugging code.
It is a common thing.
DEBUG_KERNEL is even needed for many basics debugging helpers,
for example, for FRAME_POINTERS.
So, if it would be good for SoCs...
> > >> > Would placing the strings into an __initconst array help?
> > >>
> > >> That would indeed help to reduce run-time memory consumption.
> > >
> > > Sure. We could do this. Do you want to send a patch, please?
>
> Added to my list.
>
> > >> It would not solve the raw kernel size increase.
> > >
> > > I see. Well, the compression should be pretty efficient
> > > for a text (with many spaces).
>
> My worry is not about the medium for storing the kernel image, but the
> RAM where the kernel image is loaded. The former is usually less
> restricted in size, and easier to expand, than the latter,
Well, the __initconst might be enough then.
I personally do not have any preference whether to do __initconst
or DEBUG_KERNEL or both. We should just keep it simple and
do not over engineer it.
Best Regards,
Petr
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
Hi Steven,
On Tue, Mar 2, 2021 at 3:08 PM Steven Rostedt wrote:
> On Tue, 2 Mar 2021 14:49:42 +0100
> Geert Uytterhoeven wrote:
> > > So this is basically a kernel tinyfication issue, right? Is that still
> > > pursued
> > > today? Are there better config options suitable for this than
> > > CONFIG_DEBUG_KERNEL?
> >
> > As long as I hear about products running Linux on SoCs with 10 MiB of
> > SRAM, I think the answer is yes.
> > I'm not immediately aware of a better config option. There are no more
> > TINY options left, and EXPERT selects DEBUG_KERNEL.
>
> Since the trace_printk() uses the same type of notice, I wonder if we could
> make this into a helper function and just pass in the top part.
>
> +
> 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");
>
> Only the above section is really unique. The rest can be a boiler plate.
Good idea. drivers/iommu/iommu-debugfs.c has a third copy.
> + 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");
Fortunately gcc is already smart enough to deduplicate identical strings,
but only in the same source file.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, 2 Mar 2021 at 15:35, Matthew Wilcox wrote:
>
> On Tue, Mar 02, 2021 at 03:26:50PM +0100, Marco Elver wrote:
> > +static const char no_hash_pointers_warning[9][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)
> > {
> > + const int lines[] = { 0, 1, 2, 3, 4, 5, 2, 6, 7, 8, 2, 1, 0 };
> > + int i;
> > +
> > 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("**%s**\n", no_hash_pointers_warning[lines[i]]);
>
> + for (i = 0; i < 3; i++)
> + pr_warn("**%s**\n", no_hash_pointers_warning[lines[2 - i]]);
Yeah, I had that before, but then wanted to deal with the blank line
in the middle of the thing. So I just went with the lines array above,
which seemed cleanest for dealing with the middle blank line and
footer. Or maybe there's something even nicer I missed? :-)
Thanks,
-- Marco
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, 2 Mar 2021 at 15:55, Geert Uytterhoeven wrote:
>
> Hi Marco,
>
> On Tue, Mar 2, 2021 at 3:40 PM Marco Elver wrote:
> > On Tue, 2 Mar 2021 at 15:35, Matthew Wilcox wrote:
> > > On Tue, Mar 02, 2021 at 03:26:50PM +0100, Marco Elver wrote:
> > > > +static const char no_hash_pointers_warning[9][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)
> > > > {
> > > > + const int lines[] = { 0, 1, 2, 3, 4, 5, 2, 6, 7, 8, 2, 1, 0 };
> > > > + int i;
> > > > +
> > > > 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("**%s**\n", no_hash_pointers_warning[lines[i]]);
> > >
> > > + for (i = 0; i < 3; i++)
> > > + pr_warn("**%s**\n", no_hash_pointers_warning[lines[2 -
> > > i]]);
> >
> > Yeah, I had that before, but then wanted to deal with the blank line
> > in the middle of the thing. So I just went with the lines array above,
> > which seemed cleanest for dealing with the middle blank line and
> > footer. Or maybe there's something even nicer I missed? :-)
>
> Gcc deduplicates the identical strings, so you don't have to go through
> a double indirection at all?
In this case I think we do, because we're asking the compiler to
create a giant array char[9][55]. If we had char*[9], then you're
right, but in that case we would not benefit from __initconst for the
majority of the data.
Thanks,
-- Marco
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, Mar 02, 2021 at 03:28:09PM +0100, Geert Uytterhoeven wrote:
> On Tue, Mar 2, 2021 at 3:08 PM Steven Rostedt wrote:
> > On Tue, 2 Mar 2021 14:49:42 +0100
> > Geert Uytterhoeven wrote:
> > > > So this is basically a kernel tinyfication issue, right? Is that still
> > > > pursued
> > > > today? Are there better config options suitable for this than
> > > > CONFIG_DEBUG_KERNEL?
> > >
> > > As long as I hear about products running Linux on SoCs with 10 MiB of
> > > SRAM, I think the answer is yes.
> > > I'm not immediately aware of a better config option. There are no more
> > > TINY options left, and EXPERT selects DEBUG_KERNEL.
> >
> > Since the trace_printk() uses the same type of notice, I wonder if we could
> > make this into a helper function and just pass in the top part.
> >
> > +
> > 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");
> >
> > Only the above section is really unique. The rest can be a boiler plate.
>
> Good idea. drivers/iommu/iommu-debugfs.c has a third copy.
+1. Let's keep it in some helper that can be added if we have a corresponding
functionality.
> > + 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");
>
> Fortunately gcc is already smart enough to deduplicate identical strings,
> but only in the same source file.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On 02/03/2021 15.28, Geert Uytterhoeven wrote:
> Fortunately gcc is already smart enough to deduplicate identical strings,
> but only in the same source file.
Yeah, gcc can't do much more since it only handles one source file at a
time. However, the linker certainly deduplicates strings across
translation units :)
I don't think gcc bothers to do the tail merging since the linker does
it, but that may depend on optimization level and gcc version; it does
seem to merge identical strings within a TU, at least in very simple cases.
Rasmus
$ grep . *.c
a.c:const char *a1(void) { return "string"; }
a.c:const char *a2(void) { return "string"; }
a.c:const char *a3(void) { return "longer string"; }
b.c:const char *b1(void) { return "string"; }
b.c:const char *b2(void) { return "longer string"; }
b.c:const char *b3(void) { return "much longer string"; }
main.c:#include
main.c:const char *a1(void);
main.c:const char *a2(void);
main.c:const char *a3(void);
main.c:const char *b1(void);
main.c:const char *b2(void);
main.c:const char *b3(void);
main.c:int main(int argc, char *argv[])
main.c:{
main.c:#define p(x) printf("%s(): %p - [%s]\n", #x, x(), x())
main.c: p(a1);
main.c: p(a2);
main.c: p(a3);
main.c: p(b1);
main.c: p(b2);
main.c: p(b3);
main.c:
main.c: return 0;
main.c:}
$ gcc -O2 -o a.o -c a.c ; gcc -O2 -o b.o -c b.c ; gcc -O2 -o main.o -c
main.c ; gcc -o main main.o a.o b.o ; ./main
a1(): 0x560b1bc3d033 - [string]
a2(): 0x560b1bc3d033 - [string]
a3(): 0x560b1bc3d02c - [longer string]
b1(): 0x560b1bc3d033 - [string]
b2(): 0x560b1bc3d02c - [longer string]
b3(): 0x560b1bc3d027 - [much longer string]
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
Hi Marco,
On Tue, Mar 2, 2021 at 3:40 PM Marco Elver wrote:
> On Tue, 2 Mar 2021 at 15:35, Matthew Wilcox wrote:
> > On Tue, Mar 02, 2021 at 03:26:50PM +0100, Marco Elver wrote:
> > > +static const char no_hash_pointers_warning[9][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)
> > > {
> > > + const int lines[] = { 0, 1, 2, 3, 4, 5, 2, 6, 7, 8, 2, 1, 0 };
> > > + int i;
> > > +
> > > 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("**%s**\n", no_hash_pointers_warning[lines[i]]);
> >
> > + for (i = 0; i < 3; i++)
> > + pr_warn("**%s**\n", no_hash_pointers_warning[lines[2 - i]]);
>
> Yeah, I had that before, but then wanted to deal with the blank line
> in the middle of the thing. So I just went with the lines array above,
> which seemed cleanest for dealing with the middle blank line and
> footer. Or maybe there's something even nicer I missed? :-)
Gcc deduplicates the identical strings, so you don't have to go through
a double indirection at all?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, Mar 02, 2021 at 03:26:50PM +0100, Marco Elver wrote:
> +static const char no_hash_pointers_warning[9][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)
> {
> + const int lines[] = { 0, 1, 2, 3, 4, 5, 2, 6, 7, 8, 2, 1, 0 };
> + int i;
> +
> 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("**%s**\n", no_hash_pointers_warning[lines[i]]);
+ for (i = 0; i < 3; i++)
+ pr_warn("**%s**\n", no_hash_pointers_warning[lines[2 - i]]);
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, Mar 02, 2021 at 09:08AM -0500, Steven Rostedt wrote:
> On Tue, 2 Mar 2021 14:49:42 +0100
> Geert Uytterhoeven wrote:
>
> > > So this is basically a kernel tinyfication issue, right? Is that still
> > > pursued
> > > today? Are there better config options suitable for this than
> > > CONFIG_DEBUG_KERNEL?
> >
> > As long as I hear about products running Linux on SoCs with 10 MiB of
> > SRAM, I think the answer is yes.
> > I'm not immediately aware of a better config option. There are no more
> > TINY options left, and EXPERT selects DEBUG_KERNEL.
>
> Since the trace_printk() uses the same type of notice, I wonder if we could
> make this into a helper function and just pass in the top part.
>
> + 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");
>
> Only the above section is really unique. The rest can be a boiler plate.
Short of procedurally generating some of these strings, I was
experimenting with the below.
Would that be reasonable?
Thanks,
-- Marco
-- >8 --
From: Marco Elver
Date: Tue, 2 Mar 2021 15:07:28 +0100
Subject: [PATCH] lib/vsprintf: reduce space taken by no_hash_pointers warning
Move the no_hash_pointers warning string into __initconst section, so
that it is discarded after init. Remove common start/end characters and
remove repeated lines from the array.
Link:
https://lkml.kernel.org/r/camuhmdulkzcjevvjcp7txzldwljsqphe8hqxhnztni9bjt_...@mail.gmail.com
Reported-by: Geert Uytterhoeven
Signed-off-by: Marco Elver
---
lib/vsprintf.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 41ddc353ebb8..1e63b43955f6 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2094,23 +2094,27 @@ 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[9][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)
{
+ const int lines[] = { 0, 1, 2, 3, 4, 5, 2, 6, 7, 8, 2, 1, 0 };
+ int i;
+
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("**%s**\n", no_hash_pointers_warning[lines[i]]);
return 0;
}
--
2.30.1.766.gb4fecdf3b7-goog
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, 2 Mar 2021 14:49:42 +0100
Geert Uytterhoeven wrote:
> > So this is basically a kernel tinyfication issue, right? Is that still
> > pursued
> > today? Are there better config options suitable for this than
> > CONFIG_DEBUG_KERNEL?
>
> As long as I hear about products running Linux on SoCs with 10 MiB of
> SRAM, I think the answer is yes.
> I'm not immediately aware of a better config option. There are no more
> TINY options left, and EXPERT selects DEBUG_KERNEL.
Since the trace_printk() uses the same type of notice, I wonder if we could
make this into a helper function and just pass in the top part.
+ 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");
Only the above section is really unique. The rest can be a boiler plate.
-- Steve
+ 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");
+
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
Hi Vlastimil, Petr,
On Tue, Mar 2, 2021 at 2:37 PM Vlastimil Babka wrote:
> On 3/2/21 2:29 PM, Petr Mladek wrote:
> > On Tue 2021-03-02 13:51:35, Geert Uytterhoeven wrote:
> >> > > > +
> >> > > > +
> >> > > > 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");
> >> > > > +
> >> > > > + return 0;
> >> > > > +}
> >> > > > +early_param("no_hash_pointers", no_hash_pointers_enable);
> >> > >
> >> > > While bloat-o-meter is not smart enough to notice the real size impact,
> >> > > this does add more than 500 bytes of string data to the kernel.
> >> > > Do we really need such a large message?
> >> > > Perhaps the whole no_hash_pointers machinery should be protected by
> >> > > "#ifdef CONFIG_DEBUG_KERNEL"?
> >
> > This was the deal. The configure option is a no-go, see below and also
> > https://lore.kernel.org/r/CAHk-=wgaK4cz=k-jb4p-kpxbv73m9bja2w1w1lr3iu8+nep...@mail.gmail.com
>
> I think it's a no-go only when enabling such option equals to
> "no_hash_pointers"
> being always passed. What Geert suggests is that you need both
> CONFIG_DEBUG_KERNEL *and* no_hash_pointers and that's different.
Exactly.
> >> > We recently stumbled across this, and it appears an increasing number
> >> > of production kernels enable CONFIG_DEBUG_KERNEL [1], so it likely
> >> > isn't the solution (we tried to use CONFIG_DEBUG_KERNEL in similar
> >>
> >> I guess the people who do care about kernel size do know to disable
> >> CONFIG_DEBUG_KERNEL, so it would help them.
> >> The everything-but-the-kitchen-sink distro people don't care about kernel
> >> size anyway.
> >
> > The problem with the configure option is not about size. The problem is
> > that there would be many kernels in the wild with this option enabled.
> > People would install them without knowing that they are less secure.
>
> Same as above.
>
> > Distros would need to provide both kernels. Well, they already do.
> > But it might be worse. Some distros might even want to enable it
> > by default.
> >
> > Also many bugs might be debugged without this option. Some bugs
> > are hard to reproduce and might be visible only on production
> > systems. It should be possible to enable this only when really
> > needed and the user must be aware of the risk.
>
> So this is basically a kernel tinyfication issue, right? Is that still pursued
> today? Are there better config options suitable for this than
> CONFIG_DEBUG_KERNEL?
As long as I hear about products running Linux on SoCs with 10 MiB of
SRAM, I think the answer is yes.
I'm not immediately aware of a better config option. There are no more
TINY options left, and EXPERT selects DEBUG_KERNEL.
> >> > Would placing the strings into an __initconst array help?
> >>
> >> That would indeed help to reduce run-time memory consumption.
> >
> > Sure. We could do this. Do you want to send a patch, please?
Added to my list.
> >> It would not solve the raw kernel size increase.
> >
> > I see. Well, the compression should be pretty efficient
> > for a text (with many spaces).
My worry is not about the medium for storing the kernel image, but the
RAM where the kernel image is loaded. The former is usually less
restricted in size, and easier to expand, than the latter,
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On 3/2/21 2:29 PM, Petr Mladek wrote:
> On Tue 2021-03-02 13:51:35, Geert Uytterhoeven wrote:
>> > > > +
>> > > > +
>> > > > 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");
>> > > > +
>> > > > + return 0;
>> > > > +}
>> > > > +early_param("no_hash_pointers", no_hash_pointers_enable);
>> > >
>> > > While bloat-o-meter is not smart enough to notice the real size impact,
>> > > this does add more than 500 bytes of string data to the kernel.
>> > > Do we really need such a large message?
>> > > Perhaps the whole no_hash_pointers machinery should be protected by
>> > > "#ifdef CONFIG_DEBUG_KERNEL"?
>
> This was the deal. The configure option is a no-go, see below and also
> https://lore.kernel.org/r/CAHk-=wgaK4cz=k-jb4p-kpxbv73m9bja2w1w1lr3iu8+nep...@mail.gmail.com
I think it's a no-go only when enabling such option equals to "no_hash_pointers"
being always passed. What Geert suggests is that you need both
CONFIG_DEBUG_KERNEL *and* no_hash_pointers and that's different.
>> > We recently stumbled across this, and it appears an increasing number
>> > of production kernels enable CONFIG_DEBUG_KERNEL [1], so it likely
>> > isn't the solution (we tried to use CONFIG_DEBUG_KERNEL in similar
>>
>> I guess the people who do care about kernel size do know to disable
>> CONFIG_DEBUG_KERNEL, so it would help them.
>> The everything-but-the-kitchen-sink distro people don't care about kernel
>> size anyway.
>
> The problem with the configure option is not about size. The problem is
> that there would be many kernels in the wild with this option enabled.
> People would install them without knowing that they are less secure.
Same as above.
> Distros would need to provide both kernels. Well, they already do.
> But it might be worse. Some distros might even want to enable it
> by default.
>
> Also many bugs might be debugged without this option. Some bugs
> are hard to reproduce and might be visible only on production
> systems. It should be possible to enable this only when really
> needed and the user must be aware of the risk.
So this is basically a kernel tinyfication issue, right? Is that still pursued
today? Are there better config options suitable for this than
CONFIG_DEBUG_KERNEL?
>> > Would placing the strings into an __initconst array help?
>>
>> That would indeed help to reduce run-time memory consumption.
>
> Sure. We could do this. Do you want to send a patch, please?
>
>> It would not solve the raw kernel size increase.
>
> I see. Well, the compression should be pretty efficient
> for a text (with many spaces).
>
> Best Regards,
> Petr
>
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue 2021-03-02 13:51:35, Geert Uytterhoeven wrote:
> Hi Marco,
>
> On Tue, Mar 2, 2021 at 1:45 PM Marco Elver wrote:
> > On Tue, 2 Mar 2021 at 12:51, Geert Uytterhoeven
> > wrote:
> > > On Sun, Feb 14, 2021 at 5:17 PM Timur Tabi wrote:
> > > > If the no_hash_pointers command line parameter is set, then
> > > > printk("%p") will print pointers as unhashed, which is useful for
> > > > debugging purposes. This change applies to any function that uses
> > > > vsprintf, such as print_hex_dump() and seq_buf_printf().
> > > >
> > > > A large warning message is displayed if this option is enabled.
> > > > Unhashed pointers expose kernel addresses, which can be a security
> > > > risk.
> > > >
> > > > --- a/lib/vsprintf.c
> > > > +++ b/lib/vsprintf.c
> > > > @@ -2090,6 +2090,32 @@ char *fwnode_string(char *buf, char *end, struct
> > > > fwnode_handle *fwnode,
> > > > return widen_string(buf, buf - buf_start, end, spec);
> > > > }
> > > >
> > > > +/* Disable pointer hashing if requested */
> > > > +bool no_hash_pointers __ro_after_init;
> > > > +EXPORT_SYMBOL_GPL(no_hash_pointers);
> > > > +
> > > > +static int __init no_hash_pointers_enable(char *str)
> > > > +{
> > > > + 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");
> > > > +
> > > > + return 0;
> > > > +}
> > > > +early_param("no_hash_pointers", no_hash_pointers_enable);
> > >
> > > While bloat-o-meter is not smart enough to notice the real size impact,
> > > this does add more than 500 bytes of string data to the kernel.
> > > Do we really need such a large message?
> > > Perhaps the whole no_hash_pointers machinery should be protected by
> > > "#ifdef CONFIG_DEBUG_KERNEL"?
This was the deal. The configure option is a no-go, see below and also
https://lore.kernel.org/r/CAHk-=wgaK4cz=k-jb4p-kpxbv73m9bja2w1w1lr3iu8+nep...@mail.gmail.com
> > We recently stumbled across this, and it appears an increasing number
> > of production kernels enable CONFIG_DEBUG_KERNEL [1], so it likely
> > isn't the solution (we tried to use CONFIG_DEBUG_KERNEL in similar
>
> I guess the people who do care about kernel size do know to disable
> CONFIG_DEBUG_KERNEL, so it would help them.
> The everything-but-the-kitchen-sink distro people don't care about kernel
> size anyway.
The problem with the configure option is not about size. The problem is
that there would be many kernels in the wild with this option enabled.
People would install them without knowing that they are less secure.
Distros would need to provide both kernels. Well, they already do.
But it might be worse. Some distros might even want to enable it
by default.
Also many bugs might be debugged without this option. Some bugs
are hard to reproduce and might be visible only on production
systems. It should be possible to enable this only when really
needed and the user must be aware of the risk.
> > Would placing the strings into an __initconst array help?
>
> That would indeed help to reduce run-time memory consumption.
Sure. We could do this. Do you want to send a patch, please?
> It would not solve the raw kernel size increase.
I see. Well, the compression should be pretty efficient
for a text (with many spaces).
Best Regards,
Petr
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
Hi Marco,
On Tue, Mar 2, 2021 at 1:45 PM Marco Elver wrote:
> On Tue, 2 Mar 2021 at 12:51, Geert Uytterhoeven wrote:
> > On Sun, Feb 14, 2021 at 5:17 PM Timur Tabi wrote:
> > > If the no_hash_pointers command line parameter is set, then
> > > printk("%p") will print pointers as unhashed, which is useful for
> > > debugging purposes. This change applies to any function that uses
> > > vsprintf, such as print_hex_dump() and seq_buf_printf().
> > >
> > > A large warning message is displayed if this option is enabled.
> > > Unhashed pointers expose kernel addresses, which can be a security
> > > risk.
> > >
> > > Also update test_printf to skip the hashed pointer tests if the
> > > command-line option is set.
> > >
> > > Signed-off-by: Timur Tabi
> >
> > Thanks for your patch, which is now commit 5ead723a20e0447b
> > ("lib/vsprintf: no_hash_pointers prints all addresses as unhashed") in
> > v5.12-rc1.
> >
> > > --- a/lib/vsprintf.c
> > > +++ b/lib/vsprintf.c
> > > @@ -2090,6 +2090,32 @@ char *fwnode_string(char *buf, char *end, struct
> > > fwnode_handle *fwnode,
> > > return widen_string(buf, buf - buf_start, end, spec);
> > > }
> > >
> > > +/* Disable pointer hashing if requested */
> > > +bool no_hash_pointers __ro_after_init;
> > > +EXPORT_SYMBOL_GPL(no_hash_pointers);
> > > +
> > > +static int __init no_hash_pointers_enable(char *str)
> > > +{
> > > + 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");
> > > +
> > > + return 0;
> > > +}
> > > +early_param("no_hash_pointers", no_hash_pointers_enable);
> >
> > While bloat-o-meter is not smart enough to notice the real size impact,
> > this does add more than 500 bytes of string data to the kernel.
> > Do we really need such a large message?
> > Perhaps the whole no_hash_pointers machinery should be protected by
> > "#ifdef CONFIG_DEBUG_KERNEL"?
>
> We recently stumbled across this, and it appears an increasing number
> of production kernels enable CONFIG_DEBUG_KERNEL [1], so it likely
> isn't the solution (we tried to use CONFIG_DEBUG_KERNEL in similar
I guess the people who do care about kernel size do know to disable
CONFIG_DEBUG_KERNEL, so it would help them.
The everything-but-the-kitchen-sink distro people don't care about kernel
size anyway.
> way, and it wasn't reliable). Having no_hash_pointers frees us of
> having to rely on CONFIG_DEBUG_KERNEL. (Perhaps somebody else will
> comment, but I believe there were strong objections to making the
> pointer hashing dependent on more Kconfig options.)
>
> [1] https://lkml.kernel.org/r/20210223082043.1972742-1-el...@google.com
>
> Would placing the strings into an __initconst array help?
That would indeed help to reduce run-time memory consumption.
It would not solve the raw kernel size increase.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
On Tue, 2 Mar 2021 at 12:51, Geert Uytterhoeven wrote:
> Hi Timur,
>
> On Sun, Feb 14, 2021 at 5:17 PM Timur Tabi wrote:
> > If the no_hash_pointers command line parameter is set, then
> > printk("%p") will print pointers as unhashed, which is useful for
> > debugging purposes. This change applies to any function that uses
> > vsprintf, such as print_hex_dump() and seq_buf_printf().
> >
> > A large warning message is displayed if this option is enabled.
> > Unhashed pointers expose kernel addresses, which can be a security
> > risk.
> >
> > Also update test_printf to skip the hashed pointer tests if the
> > command-line option is set.
> >
> > Signed-off-by: Timur Tabi
>
> Thanks for your patch, which is now commit 5ead723a20e0447b
> ("lib/vsprintf: no_hash_pointers prints all addresses as unhashed") in
> v5.12-rc1.
>
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -2090,6 +2090,32 @@ char *fwnode_string(char *buf, char *end, struct
> > fwnode_handle *fwnode,
> > return widen_string(buf, buf - buf_start, end, spec);
> > }
> >
> > +/* Disable pointer hashing if requested */
> > +bool no_hash_pointers __ro_after_init;
> > +EXPORT_SYMBOL_GPL(no_hash_pointers);
> > +
> > +static int __init no_hash_pointers_enable(char *str)
> > +{
> > + 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");
> > +
> > + return 0;
> > +}
> > +early_param("no_hash_pointers", no_hash_pointers_enable);
>
> While bloat-o-meter is not smart enough to notice the real size impact,
> this does add more than 500 bytes of string data to the kernel.
> Do we really need such a large message?
> Perhaps the whole no_hash_pointers machinery should be protected by
> "#ifdef CONFIG_DEBUG_KERNEL"?
We recently stumbled across this, and it appears an increasing number
of production kernels enable CONFIG_DEBUG_KERNEL [1], so it likely
isn't the solution (we tried to use CONFIG_DEBUG_KERNEL in similar
way, and it wasn't reliable). Having no_hash_pointers frees us of
having to rely on CONFIG_DEBUG_KERNEL. (Perhaps somebody else will
comment, but I believe there were strong objections to making the
pointer hashing dependent on more Kconfig options.)
[1] https://lkml.kernel.org/r/20210223082043.1972742-1-el...@google.com
Would placing the strings into an __initconst array help?
Thanks,
-- Marco
Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
Hi Timur,
On Sun, Feb 14, 2021 at 5:17 PM Timur Tabi wrote:
> If the no_hash_pointers command line parameter is set, then
> printk("%p") will print pointers as unhashed, which is useful for
> debugging purposes. This change applies to any function that uses
> vsprintf, such as print_hex_dump() and seq_buf_printf().
>
> A large warning message is displayed if this option is enabled.
> Unhashed pointers expose kernel addresses, which can be a security
> risk.
>
> Also update test_printf to skip the hashed pointer tests if the
> command-line option is set.
>
> Signed-off-by: Timur Tabi
Thanks for your patch, which is now commit 5ead723a20e0447b
("lib/vsprintf: no_hash_pointers prints all addresses as unhashed") in
v5.12-rc1.
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2090,6 +2090,32 @@ char *fwnode_string(char *buf, char *end, struct
> fwnode_handle *fwnode,
> return widen_string(buf, buf - buf_start, end, spec);
> }
>
> +/* Disable pointer hashing if requested */
> +bool no_hash_pointers __ro_after_init;
> +EXPORT_SYMBOL_GPL(no_hash_pointers);
> +
> +static int __init no_hash_pointers_enable(char *str)
> +{
> + 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");
> +
> + return 0;
> +}
> +early_param("no_hash_pointers", no_hash_pointers_enable);
While bloat-o-meter is not smart enough to notice the real size impact,
this does add more than 500 bytes of string data to the kernel.
Do we really need such a large message?
Perhaps the whole no_hash_pointers machinery should be protected by
"#ifdef CONFIG_DEBUG_KERNEL"?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all addresses as unhashed
If the no_hash_pointers command line parameter is set, then
printk("%p") will print pointers as unhashed, which is useful for
debugging purposes. This change applies to any function that uses
vsprintf, such as print_hex_dump() and seq_buf_printf().
A large warning message is displayed if this option is enabled.
Unhashed pointers expose kernel addresses, which can be a security
risk.
Also update test_printf to skip the hashed pointer tests if the
command-line option is set.
Signed-off-by: Timur Tabi
Acked-by: Petr Mladek
Acked-by: Randy Dunlap
Acked-by: Sergey Senozhatsky
Acked-by: Vlastimil Babka
Acked-by: Marco Elver
---
.../admin-guide/kernel-parameters.txt | 15
lib/test_printf.c | 8 +
lib/vsprintf.c| 36 +--
3 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index a10b545c2070..c8993a296e71 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3281,6 +3281,21 @@
in certain environments such as networked servers or
real-time systems.
+ no_hash_pointers
+ Force pointers printed to the console or buffers to be
+ unhashed. By default, when a pointer is printed via %p
+ format string, that pointer is "hashed", i.e. obscured
+ by hashing the pointer value. This is a security
feature
+ that hides actual kernel addresses from unprivileged
+ users, but it also makes debugging the kernel more
+ difficult since unequal pointers can no longer be
+ compared. However, if this command-line option is
+ specified, then all normal pointers will have their true
+ value printed. Pointers printed via %pK may still be
+ hashed. This option should only be specified when
+ debugging the kernel. Please do not use on production
+ kernels.
+
nohibernate [HIBERNATION] Disable hibernation and resume.
nohz= [KNL] Boottime enable/disable dynamic ticks
diff --git a/lib/test_printf.c b/lib/test_printf.c
index ad2bcfa8caa1..a6755798e9e6 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -35,6 +35,8 @@ KSTM_MODULE_GLOBALS();
static char *test_buffer __initdata;
static char *alloced_buffer __initdata;
+extern bool no_hash_pointers;
+
static int __printf(4, 0) __init
do_test(int bufsize, const char *expect, int elen,
const char *fmt, va_list ap)
@@ -301,6 +303,12 @@ plain(void)
{
int err;
+ if (no_hash_pointers) {
+ pr_warn("skipping plain 'p' tests");
+ skipped_tests += 2;
+ return;
+ }
+
err = plain_hash();
if (err) {
pr_warn("plain 'p' does not appear to be hashed\n");
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3b53c73580c5..41ddc353ebb8 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2090,6 +2090,32 @@ char *fwnode_string(char *buf, char *end, struct
fwnode_handle *fwnode,
return widen_string(buf, buf - buf_start, end, spec);
}
+/* Disable pointer hashing if requested */
+bool no_hash_pointers __ro_after_init;
+EXPORT_SYMBOL_GPL(no_hash_pointers);
+
+static int __init no_hash_pointers_enable(char *str)
+{
+ 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");
+
+ return 0;
+}
+early_param("no_hash_pointers", no_hash_pointers_enable);
+
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
@@ -2297,8 +2323,14 @@ char
