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

From: Thomas-Mich Richter
Date: Tue Dec 19 2017 - 03:54:13 EST

On 12/18/2017 07:16 PM, Linus Torvalds wrote:
> 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

As Steven Rostedt already pointed out, the 2 file systems debugfs and
tracefs are accessible for root only:

[root@s35lp76 ~]# mount | egrep 'debug|trace'
debugfs on /sys/kernel/debug type debugfs (rw,relatime)
tracefs on /sys/kernel/debug/tracing type tracefs (rw,relatime)
[root@s35lp76 ~]# ls -ld /sys/kernel/debug/ /sys/kernel/debug/tracing/
drwx------ 14 root root 0 Dec 18 11:34 /sys/kernel/debug/
drwx------ 7 root root 0 Dec 18 11:34 /sys/kernel/debug/tracing/
[root@s35lp76 ~]#

Surely the file permissions for uprobe_events and uprobe_profile
in directory /sys/kernel/debug/tracing
can be changed to be accessible by owner (root) only, but does this really
I have looked at all the other files in this directory and almost
all of them have read permissions for group and others.

Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
Vorsitzende des Aufsichtsrats: Martina Koederitz
GeschÃftsfÃhrung: Dirk Wittkopp
Sitz der Gesellschaft: BÃblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294