Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <b5854fd867982527c107138d52a61010079d2321.camel@buserror.net>
From:   Scott Wood <oss@buserror.net>
To:     Kees Cook <keescook@chromium.org>, Jason Yan <yanaijie@huawei.com>
Cc:     pmladek@suse.com, rostedt@goodmis.org,
        sergey.senozhatsky@gmail.com, andriy.shevchenko@linux.intel.com,
        linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org,
        "Tobin C . Harding" <tobin@kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Daniel Axtens <dja@axtens.net>
Date:   Wed, 04 Mar 2020 15:11:39 -0600
In-Reply-To: <202003041022.26AF0178@keescook>
References: <20200304124707.22650-1-yanaijie@huawei.com>
         <202003041022.26AF0178@keescook>
Organization: Red Hat
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 2020-03-04 at 10:34 -0800, Kees Cook wrote:
> On Wed, Mar 04, 2020 at 08:47:07PM +0800, Jason Yan wrote:
> > When I am implementing KASLR for powerpc, Scott Wood argued that format
> > specifier "%p" always hashes the addresses that people do not have a
> > choice to shut it down: https://patchwork.kernel.org/cover/11367547/
> > 
> > It's true that if in a debug environment or security is not concerned,
> > such as KASLR is absent or kptr_restrict = 0,  there is no way to shut
> > the hashing down except changing the code and build the kernel again
> > to use a different format specifier like "%px". And when we want to
> > turn to security environment, the format specifier has to be changed
> > back and rebuild the kernel.
> > 
> > As KASLR is available on most popular platforms and enabled by default,
> > print the raw value of address while KASLR is absent and kptr_restrict
> > is zero. Those who concerns about security must have KASLR enabled or
> > kptr_restrict set properly.
> 
> Sorry, but no: %p censoring is there to stem the flood of endless pointer
> leaks into logs, sysfs, proc, etc. These are used for attacks to build
> reliable kernel memory targets, regardless of KASLR. (KASLR can be
> bypassed with all kinds of sampling methods, side-channels, etc.)
> 
> Linus has rejected past suggestions to provide a flag for it[1],
> wanting %p to stay unconditionally censored.

The frustration is with the inability to set a flag to say, "I'm debugging and
don't care about leaks... in fact I'd like as much information as possible to
leak to me."  Hashed addresses only allow comparison to other hashes which
doesn't help when looking at (or trying to find) data structures in kernel
memory, etc.  I get what Linus is saying about "you can't have nice things
because people won't test the normal config" but it's still annoying. :-P

In any case, this came up now due to a question about what to use when
printing crash dumps.  PowerPC currently prints stack and return addresses
with %lx (in addition to %pS in the latter case) and someone proposed
converting them to %p and/or removing them altogether.  Is there a consensus
on whether crash dumps need to be sanitized of this stuff as well?  It seems
like you'd have the addresses in the register dump as well (please don't take
that away too...).  Maybe crash dumps would be a less problematic place to
make the hashing conditional (i.e. less likely to break something in userspace
that wasn't expecting a hash)?

-Scott