Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk()

From	Andy Lutomirski <luto@amacapital.net>
Newsgroups	linux.kernel
Subject	Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk()
Date	2016-09-15 20:50 +0200
Message-ID	<shJTr-2cl-5@gated-at.bofh.it> (permalink)
References	(2 earlier) <shjPj-27U-9@gated-at.bofh.it> <shn6x-4hB-21@gated-at.bofh.it> <shngd-4l1-1@gated-at.bofh.it> <shJgK-1Z8-9@gated-at.bofh.it> <shJJL-28Z-11@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu, Sep 15, 2016 at 11:37 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Thu, Sep 15, 2016 at 11:04:47AM -0700, Andy Lutomirski wrote:
>> On Wed, Sep 14, 2016 at 11:35 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> > On Wed, Sep 14, 2016 at 11:22:00AM -0700, Andy Lutomirski wrote:
>> >> On Wed, Sep 14, 2016 at 7:55 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> >> > On Tue, Sep 13, 2016 at 02:29:28PM -0700, Andy Lutomirski wrote:
>> >> >> This will prevent a crash if the target task dies before or while
>> >> >> dumping its stack once we start freeing task stacks early.
>> >> >>
>> >> >> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> >> >
>> >> > Do we need a similar patch for show_stack()?
>> >>
>> >> Probably.  Shouldn't it go in show_stack_log_lvl() instead, though?
>> >
>> > Yeah, that would probably be better.
>>
>> This code is a colossal mess.  I really hope that, some day, we can
>> clarify which entry points are used only in dumpstack*.c and which are
>> used elsewhere.  Creating an arch/x86/kernel/dumpstack.h or just
>> merging the three files and removing all the intermediate crap from
>> the headers could help a lot.
>
> Agreed, it's a mess, though it's improving with some of my changes.
> dumpstack_32.c and dumpstack_64.c will be shrinking, with dump_trace()
> getting removed.  And they'll be shrinking even more with Linus's
> suggestion to remove show_stack_log_lvl().  Then maybe we can look at
> merging those two files into dumpstack.c with an #ifdef to separate the
> subarch differences.
>

I also wouldn't mind trying to do something to prevent ever dumping
the stack of an actively running task.  It's definitely safe to dump:

 - current

 - any task that's stopped via ptrace, etc

 - any task on the current CPU if running atomically enough that the
task can't migrate (which probably covers the nasty NMI cases, I hope)

What's *not* safe AFAIK is /proc/PID/stack.  I don't know if we can
somehow fix that short of actually sending an interrupt or NMI to
freeze the task if it's running.  I'm also not sure it's worth
worrying about it.

--Andy

Thread

[PATCH 00/12] thread_info cleanups and stack caching Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:30 +0200
  [PATCH 03/12] um: Stop conflating task_struct::stack with thread_info Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:30 +0200
    Re: [PATCH 03/12] um: Stop conflating task_struct::stack with  thread_info Ingo Molnar <mingo@kernel.org> - 2016-09-15 08:30 +0200
    [tip:x86/asm] um/Stop conflating task_struct::stack with  thread_info tip-bot for Linus Torvalds <tipbot@zytor.com> - 2016-09-15 12:50 +0200
  [PATCH 12/12] fork: Cache two thread stacks per cpu if CONFIG_VMAP_STACK is set Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
  [PATCH 01/12] x86/asm: Move 'status' from struct thread_info to struct thread_struct Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [tip:x86/asm] x86/asm: Move the thread_info::status field to  thread_struct tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-09-15 12:50 +0200
  [PATCH 10/12] lib/syscall: Pin the task stack in collect_syscall() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
  [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-14 17:00 +0200
      Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-14 20:30 +0200
        Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-14 20:40 +0200
          Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-15 20:10 +0200
            Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-15 20:40 +0200
              Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-15 20:50 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-15 21:20 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Peter Zijlstra <peterz@infradead.org> - 2016-09-16 09:50 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-16 17:20 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@kernel.org> - 2016-09-16 17:40 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Peter Zijlstra <peterz@infradead.org> - 2016-09-16 18:40 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Peter Zijlstra <peterz@infradead.org> - 2016-09-16 17:40 +0200
    Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Ingo Molnar <mingo@kernel.org> - 2016-09-15 08:40 +0200
      Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Ingo Molnar <mingo@kernel.org> - 2016-09-15 11:30 +0200
  [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Jann Horn <jann@thejh.net> - 2016-09-17 04:10 +0200
      Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski <luto@amacapital.net> - 2016-09-23 00:50 +0200
        Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski <luto@amacapital.net> - 2016-09-23 01:00 +0200
        Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Jann Horn <jann@thejh.net> - 2016-09-23 09:50 +0200
  [PATCH 06/12] sched: Add try_get_task_stack() and put_task_stack() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
  [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    Re: [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info() Ingo Molnar <mingo@kernel.org> - 2016-09-15 08:30 +0200
    [tip:x86/asm] x86/entry: Get rid of pt_regs_to_thread_info() tip-bot for Linus Torvalds <tipbot@zytor.com> - 2016-09-15 12:50 +0200
  [PATCH 05/12] x86: Move thread_info into task_struct Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [tip:x86/asm] x86: Move thread_info into task_struct tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-09-15 12:50 +0200
  [PATCH 04/12] sched: Allow putting thread_info into task_struct Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [tip:x86/asm] sched/core: Allow putting thread_info into  task_struct tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-09-15 12:50 +0200
  [PATCH 11/12] sched: Free the stack early if CONFIG_THREAD_INFO_IN_TASK Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
  [PATCH 07/12] kthread: to_live_kthread() needs try_get_task_stack() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200

csiph-web