Re: [PATCH V11 3/5] printk: hash addresses printed with %p

Hi Tobin,

On Tue, Dec 5, 2017 at 9:44 PM, Tobin C. Harding [off-list ref] wrote:

On Tue, Dec 05, 2017 at 09:20:57PM +0100, Geert Uytterhoeven wrote:

quoted

On Wed, Nov 29, 2017 at 3:05 AM, Tobin C. Harding [off-list ref] wrote:

quoted

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.

quoted

--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c

quoted

+/* 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.

We'd have to keep the lowest 4 since we are printing in hex, right? This
is easy enough to add. I wasn't the architect behind the hashing but I
can do up a patch and see if anyone who knows crypto objects.

Lowest 3 is good enough for all natural types, up to long long.
We may still receive complaints from people who care about seeing if
a pointer is cacheline-aligned or not. Fixing that may need up to 7 bits, I'm
afraid, which is a bit too much to give up.

Gr{oetje,eeting}s,

                        Geert