Messages in this thread | | | From | Michael Ellerman <> | Subject | Re: Long live %pK (was Re: [PATCH tip/core/rcu 02/20] torture: Prepare scripting for shift from %p to %pK) | Date | Wed, 13 Dec 2017 23:59:46 +1100 |
| |
Linus Torvalds <torvalds@linux-foundation.org> 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@ellerman.id.au> 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
| |