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

[Michael Ellerman]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> This is a perfect example of just %pK being complete shit.
>
> %pK doesn't actually do any file permissions right. It looks like it does
> it, but it's just a hot mess of garbage.
>
> And %pK doesn't even work the way you claim it does. Not in the general
> case, and only with a particular value.

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.

> On Dec 11, 2017 21:26, "Michael Ellerman" <mpe@xxxxxxxxxxxxxx> wrote: I
> >
> >
> > I understand that the CAP_SYSLOG checking that %pK does is kind of
> > gross, but it does work in at least some useful cases like this.
> >
> > What am I missing?
>
>
> Just do the damn thing right, like /proc/kallsyms does these days.
>
> With the proper open time cred check, not the wrong one at io time.

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.

> Which has the added advantage that it actually does the right thing even
> when you don't have kptr_restrict set, or when you have patches to make it
> print zero even for people with capabilities.
>
> Don't depend on some random flag that has nothing to do with your actual
> example and that has random values for security.

> Just say no to kptr_restrict "logic". Your example basically depends
> entirely on one particular setting, when (a) real distributions have a
> different value and expose those pointers that your claim shouldn't be
> exposed and (b) other people are pushing for values that will hide the
> values that you claim area needed.

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.

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.

I can't think of any other flag or setting to sensibly determine if
vmallocinfo should be visible, so I've just done:

	if (kptr_restrict < 2 && has_capability_noaudit(current, CAP_SYSLOG))
		priv->show_addrs = true;
	else
		priv->show_addrs = false;

So basically visible to root, unless kptr_restrict == 2, otherwise not
visible. 

cheers