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

[Jann Horn]

On Fri, Sep 30, 2016 at 10:58:56AM -0700, Andy Lutomirski wrote:
> 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)
 - read the remote thread's saved kernel stack pointer
 - checks that the saved kernel stack pointer points to the main part of
   the remote stack
 - read a frame pointer through the saved kernel stack pointer; the read
   value could be anything if a race occured
 - check that the frame pointer points into the main part of the remote
   stack
 - read a saved instruction pointer through the frame pointer; the read
   value could be anything if a race occured
 - ensure that the "saved instruction pointer" doesn't point to the
   sections .sched.text and .spinlock.text
 - return the "saved instruction pointer"

So as far as I can tell, it *might* be possible for userspace to leak
information about the kernel text by alternatingly storing a proper
saved instruction pointer and a user-supplied value in a place from
which the kernel tries to read a saved instruction pointer.
By supplying values that are suspected to point to kernel text and
looking at the reported wchan values, an attacker could learn information
that allows him to e.g. defeat kASLR and even slowly locate functions in
custom kernels.

I vaguely remember seeing some similar code that defends against issues
like this by reading some counter beforehand and afterwards and checking
that it stays the same, but I'm not sure where that was. I think I saw it
while looking at the series for vmalloc()ed stacks.

Attachment: signature.asc
Description: Digital signature