Re: Long live %pK (was Re: [PATCH tip/core/rcu 02/20] torture: Prepare scripting for shift from %p to %pK)

From: Linus Torvalds
Date: Wed Dec 13 2017 - 13:21:34 EST


On Wed, Dec 13, 2017 at 4:59 AM, Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:
>
> Right. My email was only about the kptr_restrict = 1 case, but I didn't
> actually make that clear.
>
> But that's also sort of my point, it has multiple modes of operation,
> which is useful.

No it isn't.

It's completely useless. Let me count the ways:

- it ties a lot of completely unrelated things together in illogical
ways. What if you want vmallocinfo for debuggability, but not
something else?

- we've had it for most of a decade, and few people use it, and it's
not actually fixed any real security holes. It has only helped on
particular setups, not in general.

> OK, that's the piece I was missing - ie. what to do in the case where
> %px for all users is too permissive but %p is not useful.

Right. THAT IS THE WHOLE POINT OF THE NEW %p BEHAVIOR.

Make people actually _think_ about the things they see, and hopefully
just remove it entirely. Not the total idiocy that was %pK that never
resulted in any actual improvement anywhere.

%pK was the "sprinkle some crack on him, let's get out of here"
approach to security. It's bogus shit. Don't do it.

Seriously. It's been sprinkled around randomly just to make random
people feel like they did something. IT DID NOTHING.

The 'K' literally stands for "krack". Because spelling wasn't the
strong part of the thing either.

> I'm still a bit confused by the above. Because kallsyms which is your
> example of how to do it right, still uses kptr_restrict. I get that it
> also checks kallsyms_for_perf(), but that's only in the
> kptr_restrict = 0 case.

Yeah, it was probably a mistake, but I didn't want to change the old behavior.

> Anyway, I'll do a patch for vmallocinfo to do the CAP_SYSLOG check at
> open time, and use that to decide if it should print 0 or the address.

.. don't use CAP_SYSLOG, at least without thihnking about it. That
was just another mistake of mine in thinking that "let's keep the old
behavior" is a good idea.

Seriously, what does CAP_SYSLOG have to do with kernel address
debugging? Nothing, really.

I suspect CAP_SYS_ADMIN is a much saner thing to use.

Ask yourself: who really should get access to vmalloc addresses?

Linus