Re: [PATCH 1/3] proc: Stop reporting eip and esp in /proc/PID/stat

[cc: PeterZ]

On Fri, Sep 30, 2016 at 11:56 AM, Jann Horn [off-list ref] wrote:

On Fri, Sep 30, 2016 at 10:58:56AM -0700, Andy Lutomirski wrote:

quoted

Reporting these fields on a non-current task is dangerous.  If the
task is in any state other than normal kernel code, they may contain
garbage or even kernel addresses on some architectures.

Stupid question: Doesn't something similar apply to wchan?
Am I missing something?

It looks to me as if the get_wchan() implementation of X86 has the same
issue, with the difference that it's not obviously usable as an infoleak
because only known symbol names are printed, not numeric values.

get_wchan() basically does the following:

 - make sure the remote thread is sleeping (but don't take any locks to
   ensure it stays that way)

Peter, how nasty would it be to add some lightish-weight lock that
lets us pin a task in a non-running state?  Maybe we could take the rq
lock, do something to the task to make it sleepy (steal it off the
queue?), unlock the lock, do whatever we're going, then take the lock
again and put it back.

Or if we had a seqlock-like thing, we could maybe arrange for
get_wchan to abort if the task get scheduled between when it starts
and when it finishes.

On an unrelated note, can we please lock down all the silly historical
*userspace* info leaks in /proc?  Nasty ones include: net, cmdline (at
the very least, only argv[0] should be visible if the reader lacks
ptrace access).

Less nasty ones include: limits, sched, autogroup, comm, wchan,
schedstat, cpuset, cgroup, oom_*, sessionid, coredump_filter

uid_map, gid_map, etc are just screwed up.  They should be per
*namespace* somewhere, and they should require creds on the namespace.

timerslack is totally fscked up -- it allows ugo to write and it
checks the wrong creds.  Jann, does your series fix that?

--Andy