From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Subject: Re: [PATCH V11 3/5] printk: hash addresses printed with %p Date: Tue, 5 Dec 2017 21:20:57 +0100 Message-ID: References: <1511921105-3647-1-git-send-email-me@tobin.cc> <1511921105-3647-4-git-send-email-me@tobin.cc> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: kernel-hardening@lists.openwall.com, Linus Torvalds , "Jason A. Donenfeld" , "Theodore Ts'o" , Kees Cook , Paolo Bonzini , Tycho Andersen , "Roberts, William C" , Tejun Heo , Jordan Glover , Greg KH , Petr Mladek , Joe Perches , Ian Campbell , Sergey Senozhatsky , Catalin Marinas , Will Deacon , Steven Rostedt , Chris Fries , Dave Weinstein To: "Tobin C. Harding" Return-path: Received: from mail-qt0-f195.google.com ([209.85.216.195]:37335 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751432AbdLEUU7 (ORCPT ); Tue, 5 Dec 2017 15:20:59 -0500 In-Reply-To: <1511921105-3647-4-git-send-email-me@tobin.cc> Sender: netdev-owner@vger.kernel.org List-ID: Hi Tobin, On Wed, Nov 29, 2017 at 3:05 AM, Tobin C. Harding wrote: > Currently there exist approximately 14 000 places in the kernel where > addresses are being printed using an unadorned %p. This potentially > leaks sensitive information regarding the Kernel layout in memory. Many > of these calls are stale, instead of fixing every call lets hash the > address by default before printing. This will of course break some > users, forcing code printing needed addresses to be updated. > > Code that _really_ needs the address will soon be able to use the new > printk specifier %px to print the address. > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > +/* Maps a pointer to a 32 bit unique identifier. */ > +static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec) > +{ > + unsigned long hashval; > + const int default_width = 2 * sizeof(ptr); > + > + if (unlikely(!have_filled_random_ptr_key)) { > + spec.field_width = default_width; > + /* string length must be less than default_width */ > + return string(buf, end, "(ptrval)", spec); > + } > + > +#ifdef CONFIG_64BIT > + hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key); > + /* > + * Mask off the first 32 bits, this makes explicit that we have > + * modified the address (and 32 bits is plenty for a unique ID). > + */ > + hashval = hashval & 0xffffffff; > +#else > + hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key); > +#endif Would it make sense to keep the 3 lowest bits of the address? Currently printed pointers no longer have any correlation with the actual alignment in memory of the object, which is a typical cause of a class of bugs. Gr{oetje,eeting}s, Geert