Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755972AbcDDPzG (ORCPT ); Mon, 4 Apr 2016 11:55:06 -0400 Received: from mx2.suse.de ([195.135.220.15]:60689 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754012AbcDDPzD (ORCPT ); Mon, 4 Apr 2016 11:55:03 -0400 Date: Mon, 4 Apr 2016 17:55:01 +0200 From: Petr Mladek To: Josh Poimboeuf Cc: Jiri Kosina , Jessica Yu , Miroslav Benes , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Vojtech Pavlik Subject: Re: [RFC PATCH v1.9 07/14] x86/stacktrace: add function for detecting reliable stack traces Message-ID: <20160404155501.GR5522@pathway.suse.cz> References: <1f8c648ed8b8eb49a75f5a6cacf8b7ca76f44fa9.1458933243.git.jpoimboe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1f8c648ed8b8eb49a75f5a6cacf8b7ca76f44fa9.1458933243.git.jpoimboe@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2271 Lines: 72 On Fri 2016-03-25 14:34:54, Josh Poimboeuf wrote: > For live patching and possibly other use cases, a stack trace is only > useful if you can be assured that it's completely reliable. Add a new > save_stack_trace_tsk_reliable() function to achieve that. > > Scenarios which indicate that a stack strace may be unreliable: > > - interrupt stacks > - preemption > - corrupted stack data > - newly forked tasks > - running tasks > - the user didn't provide a large enough entries array > > Also add a config option so arch-independent code can determine at build > time whether the function is implemented. > > diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c > index 3b10518..9c68bfc 100644 > --- a/arch/x86/kernel/dumpstack.c > +++ b/arch/x86/kernel/dumpstack.c > @@ -145,6 +145,42 @@ int print_context_stack_bp(struct thread_info *tinfo, > } > EXPORT_SYMBOL_GPL(print_context_stack_bp); > > +int print_context_stack_reliable(struct thread_info *tinfo, > + unsigned long *stack, unsigned long *bp, > + const struct stacktrace_ops *ops, > + void *data, unsigned long *end, int *graph) > +{ > + struct stack_frame *frame = (struct stack_frame *)*bp; > + struct stack_frame *last_frame = frame; I tried to debug why the patching never finishes as reported by Mirek. This initialization breaks the whole function, see below. I would initialize last_frame to NULL or maybe (void *)stack. > + unsigned long *ret_addr = &frame->return_address; > + > + if (test_ti_thread_flag(tinfo, TIF_FORK)) > + return -EINVAL; > + > + while (valid_stack_ptr(tinfo, ret_addr, sizeof(*ret_addr), end)) { > + unsigned long addr = *ret_addr; > + > + if (frame <= last_frame || !__kernel_text_address(addr) || frame == last_frame in the very rist iteration, so we always return -EINVAL. Best Regards, Petr > + in_preempt_schedule_irq(addr)) > + return -EINVAL; > + > + if (ops->address(data, addr, 1)) > + return -EINVAL; > + > + print_ftrace_graph_addr(addr, data, ops, tinfo, graph); > + > + last_frame = frame; > + frame = frame->next_frame; > + ret_addr = &frame->return_address; > + } > + > + if (last_frame + 1 != (void *)task_pt_regs(tinfo->task)) > + return -EINVAL; > + > + *bp = (unsigned long)frame; > + return 0; > +}