Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+55aFw3AZtzf6KHS4desHJ3CajKs46VQxOi6-+0U8A4y0v6ug@mail.gmail.com>
Date: Tue, 5 Dec 2017 13:22:47 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: David Laight <David.Laight@...lab.com>, Kees Cook <keescook@...omium.org>, 
	"Tobin C. Harding" <me@...in.cc>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, 
	"Jason A. Donenfeld" <Jason@...c4.com>, "Theodore Ts'o" <tytso@....edu>, Paolo Bonzini <pbonzini@...hat.com>, 
	Tycho Andersen <tycho@...ho.ws>, "Roberts, William C" <william.c.roberts@...el.com>, Tejun Heo <tj@...nel.org>, 
	Jordan Glover <Golden_Miller83@...tonmail.ch>, Greg KH <gregkh@...uxfoundation.org>, 
	Petr Mladek <pmladek@...e.com>, Joe Perches <joe@...ches.com>, Ian Campbell <ijc@...lion.org.uk>, 
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>, Catalin Marinas <catalin.marinas@....com>, 
	Will Deacon <wilal.deacon@....com>, Steven Rostedt <rostedt@...dmis.org>, 
	Chris Fries <cfries@...gle.com>, Dave Weinstein <olorin@...gle.com>, 
	Daniel Micay <danielmicay@...il.com>, Djalal Harouni <tixxdz@...il.com>, 
	Radim Krcmár <rkrcmar@...hat.com>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Network Development <netdev@...r.kernel.org>, 
	David Miller <davem@...emloft.net>, Stephen Rothwell <sfr@...b.auug.org.au>, 
	Andrey Ryabinin <aryabinin@...tuozzo.com>, Alexander Potapenko <glider@...gle.com>, 
	Dmitry Vyukov <dvyukov@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH V11 4/5] vsprintf: add printk specifier %px

On Tue, Dec 5, 2017 at 1:08 PM, Randy Dunlap <rdunlap@...radead.org> wrote:
>
> This kind of option (with default hashed) is what I was just thinking of
> after having seen a few unhelpful traces.  But then the knob might not be
> changed in time for the traces either. :(

.. I really dislike the idea of such a knob.

First off, the traces I've seen that had the new %p behavior, the
hashing didn't actually matter AT ALL. The only values that were
hashed were values that weren't actually useful for debugging the
oops.

Secondly, the notion that "we want a unhashed knob for debugging" is
exactly the wrong kind of mentality. 99% of all bug reports happen in
the wild - not on developer boxes. So by default, those bug reports
had better happen with hashing enabled, or it's all entirely
pointless.

If you have an oops that happens on your own box due to code that
you're writing yourself (and expect to debug yourself), then honestly,
the hashing is going to be the least of your issues. If you can't find
out the bug under those circumstances, and you're confused by the tiny
detail of hashing, you're doing something wrong.

So the case that matters is when an oops comes from some outside
source that won't have turned the knob off anyway.

So no. We're not adding a knob. It is fundamentally pointless.

It's not like those hex numbers were really helping people anyway.
We've turned off most of them on x86 oops reports long ago (and
entirely independently of the pointer hashing). Having stared at a lot
of oopses in my time, the only hex numbers that tend to be really
relevant are (a) the register contents (which aren't %p anyway), and
things like the faulting address (which is not, and never has been, %p
on x86, but might be on some other architecture).

Honestly, the next time anybody says "hashing makes debugging harder",
I'm going to require some actual proof of an actual oops where it
mattered that a particular value was hashed.

Not hand-waving.

Not "it surprised and confused me" because it looked different. You'll
get used to it.

So an actual "this was critical information that mattered for this
particular bug, and it was missing due to the hashing of this
particular value and debugging was harder in actual reality due to
that".

Because the actual example I have seen so far, not only didn't the
hashing matter AT ALL, most of the _unhashed_ values shouldn't have
been there either, and were due to arm still printing stuff that
shouldn't have been printed at all and just made the oops more complex
and harder to read and report.

            Linus

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.