Re: [PATCH] trace/uprobes: fix output issue with address randomization

From: Linus Torvalds
Date: Mon Dec 18 2017 - 13:16:18 EST


On Mon, Dec 18, 2017 at 9:59 AM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> Hmm, since the scrambling of %p is to prevent kernel addresses from
> leaking, I wonder if it would be OK to make it only scramble the address
> if the address is a kernel address. It should be fine to print user
> space addresses unscrambled. Shouldn't it?

So %p itself shouldn't have logic like that, because some of those
addresses can be sensitive even if they aren't strictly kernel
addresses.

For example, anything that prints out sensitive physical addresses
wouldn't look like a kernel virtual address, but it could still expose
very sensitive data.

So that check would have to be done by the user of %p, not by %p itself.

So generally, the fix for "oops %p hashing now breaks xyz" should
generally be to make sure that the protections are correct, and then
it can be turned into %px (when it can't just be removed).

In this particular case, we already have the magical case of "don't
print (null)", and I think _that_ the case that could be just extended
to say "don't print sensitive data" and then the %p can be changed
into a %px.

So one approach would be to simply checking (at _open_ time, not at IO
time! That was one of the things that I absolutely detested about %pK
- getting that fundamentally wrong) whether the opener could write a
kernel address to the file, and if the opener has those permissions,
then it obviously can read the values too.

But in this case I would suggest just making "uprobe_events" be 0600
rather than 0644. Why the hell should a normal user be able to read
whatever events that root has set?

Same goes for "uprobe_profile". Is it really sensible that anybody
can read that file? It sounds insane to me. Why should a random
regular user be able to see what probes are installed?

Honestly, very few of the "let's just change %p to %px" has been
correct. There is pretty much _always_ some permission problem or
other that says "hell no, that data should not have been so widely
available before".

The only time a pure %p -> %px is warranted is likely for oops reports
etc. We did have a couple of those.

Linus