Re: [PATCH] vsprintf: Make "null" pointer dereference more robust

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Tobin C . Harding" <me@tobin.cc>, Joe Perches <joe@perches.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>
Subject: Re: [PATCH] vsprintf: Make "null" pointer dereference more robust
Date: Wed, 7 Mar 2018 10:34:17 -0800	[thread overview]
Message-ID: <CA+55aFxr5eg_fWjyhcYFU4CSOoVP_GeB79cYa=8mUk7J1cUG7A@mail.gmail.com> (raw)
In-Reply-To: <20180307155244.b45c3fb5vcxb4q2l@pathway.suse.cz>

On Wed, Mar 7, 2018 at 7:52 AM, Petr Mladek <pmladek@suse.com> wrote:
> If we are changing things, let's do it properly. The range
> (-PAGE_SIZE,+PAGE_SIZE) is just a small subset of invalid pointers.
> Let's try to catch more of them by reading one byte using
> probe_kernel_read(). It would return -FAULT if we are not able
> to read the address but it would not crash.
>
> Then we clearly need a new message when dereferencing invalid
> poitners that are not pure NULL. I propose (efault).

NO! Absolutely f*cking NOT!

"probe_kernel_read()" is really complicated. It takes a *fault* for chrissake!

Guess what happens now to any crash report if it uses %p and there is
anything wrong with the VM?

Yes, yes, it disables page faults, but that only means that we won't
go all the way into the generic VM. We'll still take the fauly, still
do vmalloc fault filling, still do a *lot* of potentially really
complicated things.

Guys, stop this idiocy. printk() needs to be *simple* and *reliable*, not fancy.

Plus, you just made %p be an excellent leak of some very sensitive
information, like "where is the kernel mapped" etc.

So not only did you make it fundamentally more complex and fragile,
you actually made it more of a potential security issue too.

> Below is my RFC patch.

NAK NAK NAK.

Seriously. Stop this crap. Get over yourself guys.

This whole NULL discussion has been one huge pile of unbelievable *SHIT*.

The first few bytes in the address space are special, because NULL
pointers are often offset by structure offsets.  They are special
because NULL is special.

THEY ARE NOT SPECIAL BECAUSE IT CAUSES FAULTS.

The top "small negative numbers" are special, because the kernel
extensively uses the ERR_PTR model, and that is how we encode it.

THEY ARE NOT SPECIAL BECAUSE THEY CAUSE FAULTS.

Thinking that "this address causes faults is special" is moronic.

Actually testing - in something as crore and critical as printk -
whether a fault happens is beyond the pale.

                   Linus