[v2,0/6] wchan: Fix ORC support and leaky fallback

Message

Kees Cook Sept. 29, 2021, 10:02 p.m. UTC

Hi,

This attempts to solve the issues from the discussion here[1]. Specifically:

1) wchan leaking raw addresses since 152c432b128c (v5.12).

patch 1 fixes this with a revert.

2) wchan has been broken under ORC, seen as a failure to stack walk
   resulting in _usually_ a 0 value, since ee9f8fce9964 (v4.14).

patches 2-5 fixes this with Qi Zheng's new get_wchan() and changes to
the /proc code to use the new helper suggested by Peter to do the stack
walk only if the process can be kept blocked:
https://lore.kernel.org/lkml/[e-mail]/

Peter, can you take this via -tip?

Thanks!

-Kees

[1] https://lore.kernel.org/lkml/[e-mail]/

v1: https://lore.kernel.org/lkml/[e-mail]/

Kees Cook (5):
  Revert "proc/wchan: use printk format instead of lookup_symbol_name()"
  sched: Add wrapper for get_wchan() to keep task blocked
  proc: Use task_is_running() for wchan in /proc/$pid/stat
  proc: Only report /proc/$pid/wchan when process is blocked
  leaking_addresses: Always print a trailing newline

Qi Zheng (1):
  x86: Fix get_wchan() to support the ORC unwinder

 arch/x86/kernel/process.c    | 51 +++---------------------------------
 fs/proc/array.c              |  7 ++---
 fs/proc/base.c               | 20 ++++++++------
 include/linux/sched.h        |  1 +
 kernel/sched/core.c          | 16 +++++++++++
 scripts/leaking_addresses.pl |  3 ++-
 6 files changed, 36 insertions(+), 62 deletions(-)

Comments

Josh Poimboeuf Sept. 30, 2021, 1:01 a.m. UTC | #1

On Wed, Sep 29, 2021 at 03:02:12PM -0700, Kees Cook wrote:
> Hi,
> 
> This attempts to solve the issues from the discussion here[1]. Specifically:
> 
> 1) wchan leaking raw addresses since 152c432b128c (v5.12).
> 
> patch 1 fixes this with a revert.
> 
> 2) wchan has been broken under ORC, seen as a failure to stack walk
>    resulting in _usually_ a 0 value, since ee9f8fce9964 (v4.14).
> 
> patches 2-5 fixes this with Qi Zheng's new get_wchan() and changes to
> the /proc code to use the new helper suggested by Peter to do the stack
> walk only if the process can be kept blocked:
> https://lore.kernel.org/lkml/[e-mail]/
> 
> Peter, can you take this via -tip?

It all looks sane to me.  Thanks for cleaning up this mess.

- Should we use a similar sched wrapper for /proc/$pid/stack to make its
  raciness go away?

- At the risk of triggering a much larger patch set, I suspect
  get_wchan() can be made generic ;-)  It's just a glorified wrapper
  around stack_trace_save_tsk().

Regardless:

Acked-by: Josh Poimboeuf <[e-mail]>

Peter Zijlstra Sept. 30, 2021, 8:40 a.m. UTC | #2

On Wed, Sep 29, 2021 at 06:01:57PM -0700, Josh Poimboeuf wrote:

> - Should we use a similar sched wrapper for /proc/$pid/stack to make its
>   raciness go away?

Alternatively, can we make /stack go away? AFAICT the semantics of that
are far worse in that it wants the actual kernel stack of a task,
blocked or not, which is a total pain in the arse (not to mention a
giant infoleak and side-channel).

> - At the risk of triggering a much larger patch set, I suspect
>   get_wchan() can be made generic ;-)  It's just a glorified wrapper
>   around stack_trace_save_tsk().

Done that for you :-)

Kees Cook Sept. 30, 2021, 3:18 p.m. UTC | #3

On Thu, Sep 30, 2021 at 10:40:11AM +0200, Peter Zijlstra wrote:
> On Wed, Sep 29, 2021 at 06:01:57PM -0700, Josh Poimboeuf wrote:
> 
> > - Should we use a similar sched wrapper for /proc/$pid/stack to make its
> >   raciness go away?
> 
> Alternatively, can we make /stack go away? AFAICT the semantics of that
> are far worse in that it wants the actual kernel stack of a task,
> blocked or not, which is a total pain in the arse (not to mention a
> giant infoleak and side-channel).
> 
> > - At the risk of triggering a much larger patch set, I suspect
> >   get_wchan() can be made generic ;-)  It's just a glorified wrapper
> >   around stack_trace_save_tsk().
> 
> Done that for you :-)

Thanks! I wasn't sure the renaming was worth the churn, but the other
cleanups make it much nicer. :)

Do you want me to re-collect the resulting series, or do you have a tree
already for -tip for this?

-Kees

Peter Zijlstra Sept. 30, 2021, 6:39 p.m. UTC | #4

On Thu, Sep 30, 2021 at 08:18:34AM -0700, Kees Cook wrote:
> Do you want me to re-collect the resulting series, or do you have a tree
> already for -tip for this?

I stuck it here:

  https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/log/?h=sched/wchan

I'm waiting on the robots to tell I wrecked the world... :-)