Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757979AbdLRDeL (ORCPT ); Sun, 17 Dec 2017 22:34:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55154 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757825AbdLRDeJ (ORCPT ); Sun, 17 Dec 2017 22:34:09 -0500 Date: Sun, 17 Dec 2017 21:34:08 -0600 From: Josh Poimboeuf To: Andy Lutomirski Cc: Miroslav Benes , X86 ML , "linux-kernel@vger.kernel.org" , live-patching@vger.kernel.org Subject: Re: stack traces and zombie tasks Message-ID: <20171218033408.whh7bkqfuaolrldm@treble> References:

MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 18 Dec 2017 03:34:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2306 Lines: 57 On Fri, Dec 15, 2017 at 07:51:45AM -0800, Andy Lutomirski wrote: > On Fri, Dec 15, 2017 at 4:54 AM, Miroslav Benes wrote: > > Hi, > > > > commit 1959a60182f4 ("x86/dumpstack: Pin the target stack when dumping > > it") slightly changed the behaviour of stack traces dumping for zombie > > tasks. > > > > Before the commit (well, this is older SLE12 kernel, but that should not > > matter), if one called 'cat /proc//stack', they would get > > something like this > > > > [] do_exit+0x6f7/0xa80 > > [] do_group_exit+0x39/0xa0 > > [] __wake_up_parent+0x0/0x30 > > [] system_call_fastpath+0x16/0x1b > > [<00007fd128f9c4f9>] 0x7fd128f9c4f9 > > [] 0xffffffffffffffff > > > > After, one gets nothing. The trace is empty. try_get_task_stack() contains > > atomic_inc_not_zero() (CONFIG_THREAD_INFO_IN_TASK is now default on > > x86_64) and because stack_refcount is 0 for a zombie task, it returns > > NULL. Therefore, all save_stack_trace_*() functions return immediately. > > > > I guess that no one has cared about it so far. There is a problem for > > live patching though. save_stack_trace_tsk_reliable() returns -EINVAL for > > the zombie task and its stack is deemed unreliable. It could block our > > transition for quite a long time. > > > > We can skip those tasks in kernel/livepatch/ with a simple test we have in > > kGraft. Skip the task if (task->state == TASK_DEAD && task->on_cpu == 0). > > But you may want to change it generally, so better to ask first. > > > > Sounds like a bug in save_stack_trace_tsk_reliable() to me: if the > task has no stack, then the trace is 100% definitely empty :) I would agree with that, something like the following should fix it? diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 77835bc021c7..20161ef53537 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -164,8 +164,12 @@ int save_stack_trace_tsk_reliable(struct task_struct *tsk, { int ret; + /* + * If the task doesn't have a stack (e.g., a zombie), the stack is + * "reliably" empty. + */ if (!try_get_task_stack(tsk)) - return -EINVAL; + return 0; ret = __save_stack_trace_reliable(trace, tsk);