From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753447AbcGVBlq (ORCPT ); Thu, 21 Jul 2016 21:41:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35462 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753311AbcGVBln (ORCPT ); Thu, 21 Jul 2016 21:41:43 -0400 Date: Thu, 21 Jul 2016 20:41:40 -0500 From: Josh Poimboeuf To: Andy Lutomirski 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 Subject: Re: [PATCH 03/19] x86/dumpstack: remove unnecessary stack pointer arguments Message-ID: <20160722014140.nhbbhan54fhhb3k7@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.32]); Fri, 22 Jul 2016 01:41:42 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. -- Josh