linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>, X86 ML <x86@kernel.org>,
	Borislav Petkov <bp@alien8.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Brian Gerst <brgerst@gmail.com>, Jann Horn <jann@thejh.net>
Subject: Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk()
Date: Thu, 15 Sep 2016 11:41:25 -0700	[thread overview]
Message-ID: <CALCETrWG4yfaO10Pte1A0uEGpMjG6MbNfkgTaCWcsZR0eHh-YA@mail.gmail.com> (raw)
In-Reply-To: <20160915183741.wka5dzdsvbn53drp@treble>

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

  reply	other threads:[~2016-09-15 18:41 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-13 21:29 [PATCH 00/12] thread_info cleanups and stack caching Andy Lutomirski
2016-09-13 21:29 ` [PATCH 01/12] x86/asm: Move 'status' from struct thread_info to struct thread_struct Andy Lutomirski
2016-09-15 10:41   ` [tip:x86/asm] x86/asm: Move the thread_info::status field to thread_struct tip-bot for Andy Lutomirski
2016-09-13 21:29 ` [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info() Andy Lutomirski
2016-09-15  6:21   ` Ingo Molnar
2016-09-15 10:42   ` [tip:x86/asm] " tip-bot for Linus Torvalds
2016-09-13 21:29 ` [PATCH 03/12] um: Stop conflating task_struct::stack with thread_info Andy Lutomirski
2016-09-15  6:21   ` Ingo Molnar
2016-09-15 10:42   ` [tip:x86/asm] um/Stop " tip-bot for Linus Torvalds
2016-09-13 21:29 ` [PATCH 04/12] sched: Allow putting thread_info into task_struct Andy Lutomirski
2016-09-15 10:43   ` [tip:x86/asm] sched/core: " tip-bot for Andy Lutomirski
2016-09-13 21:29 ` [PATCH 05/12] x86: Move " Andy Lutomirski
2016-09-15 10:43   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2016-09-13 21:29 ` [PATCH 06/12] sched: Add try_get_task_stack() and put_task_stack() Andy Lutomirski
2016-09-13 21:29 ` [PATCH 07/12] kthread: to_live_kthread() needs try_get_task_stack() Andy Lutomirski
2016-09-13 21:29 ` [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski
2016-09-14 14:55   ` Josh Poimboeuf
2016-09-14 18:22     ` Andy Lutomirski
2016-09-14 18:35       ` Josh Poimboeuf
2016-09-15 18:04         ` Andy Lutomirski
2016-09-15 18:37           ` Josh Poimboeuf
2016-09-15 18:41             ` Andy Lutomirski [this message]
2016-09-15 19:19               ` Josh Poimboeuf
2016-09-16  7:47                 ` Peter Zijlstra
2016-09-16 15:12                   ` Andy Lutomirski
2016-09-16 15:31                     ` Peter Zijlstra
2016-09-16 15:32                       ` Andy Lutomirski
2016-09-16 16:35                         ` Peter Zijlstra
2016-09-15  6:37   ` Ingo Molnar
     [not found]     ` <CA+55aFxt=HLrELBE=BXUrWdh6LYs4gtu9S=yCruiDffq4HN80w@mail.gmail.com>
2016-09-15  9:27       ` Ingo Molnar
2016-09-13 21:29 ` [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski
2016-09-17  2:00   ` Jann Horn
2016-09-22 22:44     ` Andy Lutomirski
2016-09-22 22:50       ` Andy Lutomirski
2016-09-23  7:43       ` Jann Horn
2016-09-23 18:28         ` Kees Cook
2016-09-23 18:34           ` Jann Horn
2016-09-26  5:10             ` Tycho Andersen
2016-09-13 21:29 ` [PATCH 10/12] lib/syscall: Pin the task stack in collect_syscall() Andy Lutomirski
2016-09-13 21:29 ` [PATCH 11/12] sched: Free the stack early if CONFIG_THREAD_INFO_IN_TASK Andy Lutomirski
2016-09-13 21:29 ` [PATCH 12/12] fork: Cache two thread stacks per cpu if CONFIG_VMAP_STACK is set Andy Lutomirski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALCETrWG4yfaO10Pte1A0uEGpMjG6MbNfkgTaCWcsZR0eHh-YA@mail.gmail.com \
    --to=luto@amacapital.net \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=jann@thejh.net \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).