From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753371AbcIOSlz (ORCPT ); Thu, 15 Sep 2016 14:41:55 -0400 Received: from mail-yw0-f178.google.com ([209.85.161.178]:33530 "EHLO mail-yw0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751704AbcIOSlr (ORCPT ); Thu, 15 Sep 2016 14:41:47 -0400 MIME-Version: 1.0 In-Reply-To: <20160915183741.wka5dzdsvbn53drp@treble> References: <7cd5e328dc75d8ccd912e5783665de34503f7c63.1473801993.git.luto@kernel.org> <20160914145556.5whqcpzys7c23nib@treble> <20160914183501.cknjhsbkzxg6m6pl@treble> <20160915183741.wka5dzdsvbn53drp@treble> From: Andy Lutomirski Date: Thu, 15 Sep 2016 11:41:25 -0700 Message-ID: Subject: Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() To: Josh Poimboeuf Cc: Andy Lutomirski , X86 ML , Borislav Petkov , "linux-kernel@vger.kernel.org" , Brian Gerst , Jann Horn Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 15, 2016 at 11:37 AM, Josh Poimboeuf 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 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 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 >> >> > >> >> > 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