On Tue, 2 Mar 2021 at 12:51, Geert Uytterhoeven <[email protected]> wrote: > Hi Timur, > > On Sun, Feb 14, 2021 at 5:17 PM Timur Tabi <[email protected]> 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 <[email protected]> > > 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/[email protected] Would placing the strings into an __initconst array help? Thanks, -- Marco
