From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752018AbcGVC3f (ORCPT ); Thu, 21 Jul 2016 22:29:35 -0400 Received: from mail-vk0-f45.google.com ([209.85.213.45]:36623 "EHLO mail-vk0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751214AbcGVC3c (ORCPT ); Thu, 21 Jul 2016 22:29:32 -0400 MIME-Version: 1.0 In-Reply-To: <20160722014140.nhbbhan54fhhb3k7@treble> References: <20160722014140.nhbbhan54fhhb3k7@treble> From: Andy Lutomirski Date: Thu, 21 Jul 2016 19:29:12 -0700 Message-ID: Subject: Re: [PATCH 03/19] x86/dumpstack: remove unnecessary stack pointer arguments To: Josh Poimboeuf Cc: Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , X86 ML , "linux-kernel@vger.kernel.org" , Linus Torvalds , Steven Rostedt , Brian Gerst , Kees Cook , Peter Zijlstra , Frederic Weisbecker , Byungchul Park 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, Jul 21, 2016 at 6:41 PM, Josh Poimboeuf wrote: > On Thu, Jul 21, 2016 at 02:56:52PM -0700, Andy Lutomirski wrote: >> On Thu, Jul 21, 2016 at 2:21 PM, Josh Poimboeuf wrote: >> > When calling show_stack_log_lvl() or dump_trace() with a regs argument, >> > providing a stack pointer or frame pointer is redundant. >> > >> >> > diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c >> > index 358fe1c..c533b8b 100644 >> > --- a/arch/x86/kernel/dumpstack_32.c >> > +++ b/arch/x86/kernel/dumpstack_32.c >> > @@ -122,7 +122,7 @@ void show_regs(struct pt_regs *regs) >> > u8 *ip; >> > >> > pr_emerg("Stack:\n"); >> > - show_stack_log_lvl(NULL, regs, ®s->sp, 0, KERN_EMERG); >> > + show_stack_log_lvl(NULL, regs, NULL, 0, KERN_EMERG); >> >> This is weird -- note the &. You're at some risk of exposing a bug in >> x86_32's kernel_stack_pointer() function, which is a mess. (I don't >> see why it's written the way it is -- the actual return stack pointer >> given a pt_regs is quite well defined -- if regs->cs & 3 != 0, then >> it's regs->sp, else it's ®s->sp.) >> >> That being said, this isn't a big deal, so: >> >> Reviewed-by: Andy Lutomirski >> >> If you want to make this all a bit more reliably on x86_32, you could >> fix kernel_stack_pointer(). > > Ok. The whole '®s->sp' thing threw me for a loop. I have no idea > what kernel_stack_pointer() is trying to do. I just assumed it was > correct. I'll take a look at it and try to fix it in another patch. > On further inspection, it's probably correct except in cases of stack overflow, so I wouldn't worry about it. It's certainly overcomplicated. --Andy