Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753911AbcCHGkr (ORCPT ); Tue, 8 Mar 2016 01:40:47 -0500 Received: from mail.skyhub.de ([78.46.96.112]:56908 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751128AbcCHGki (ORCPT ); Tue, 8 Mar 2016 01:40:38 -0500 Date: Tue, 8 Mar 2016 07:40:24 +0100 From: Borislav Petkov To: Andy Lutomirski Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Oleg Nesterov , Andrew Cooper , Brian Gerst Subject: Re: [PATCH v2 10/10] x86/entry/32: Add and check a stack canary for the SYSENTER stack Message-ID: <20160308064024.GB16568@pd.tnic> References: <5a51d09b68d75fefe2c648ab869367f03b951ffa.1457243356.git.luto@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5a51d09b68d75fefe2c648ab869367f03b951ffa.1457243356.git.luto@kernel.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2269 Lines: 77 On Sat, Mar 05, 2016 at 09:52:23PM -0800, Andy Lutomirski wrote: <--- ? So all patches before are nice and verbose in explaining what they do. Where did the commit message of this one disappear? :-) Or is it that obvious that we should have a stack canary? How about some text *why* we need it? > Signed-off-by: Andy Lutomirski > --- > arch/x86/include/asm/processor.h | 3 ++- > arch/x86/kernel/process.c | 3 +++ > arch/x86/kernel/traps.c | 8 ++++++++ > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 7cd01b71b5bd..50a6dc871cc0 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -299,8 +299,9 @@ struct tss_struct { > > #ifdef CONFIG_X86_32 > /* > - * Space for the temporary SYSENTER stack: > + * Space for the temporary SYSENTER stack. > */ > + unsigned long SYSENTER_stack_canary; > unsigned long SYSENTER_stack[64]; > #endif > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > index 9f7c21c22477..ee9a9792caeb 100644 > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -57,6 +57,9 @@ __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss) = { > */ > .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, > #endif > +#ifdef CONFIG_X86_32 > + .SYSENTER_stack_canary = STACK_END_MAGIC, > +#endif > }; > EXPORT_PER_CPU_SYMBOL(cpu_tss); > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 80928ea78373..590110119e6a 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -713,6 +713,14 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code) > debug_stack_usage_dec(); > > exit: > +#if defined(CONFIG_X86_32) > + /* > + * This is the most likely code path that involves non-trivial use > + * of the SYSENTER stack. Check that we haven't overrun it. > + */ > + WARN(this_cpu_read(cpu_tss.SYSENTER_stack_canary) != STACK_END_MAGIC, > + "Overran or corrupted SYSENTER stack\n"); > +#endif > ist_exit(regs); > } > NOKPROBE_SYMBOL(do_debug); > -- > 2.5.0 > > -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply.