Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Tue, 2 Mar 2021 13:51:35 +0100 | Subject | 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 <elver@google.com> wrote: > On Tue, 2 Mar 2021 at 12:51, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Sun, Feb 14, 2021 at 5:17 PM Timur Tabi <timur@kernel.org> 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 <timur@kernel.org> > > > > 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-elver@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 -- geert@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
| |