From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934156AbdGSTpl (ORCPT ); Wed, 19 Jul 2017 15:45:41 -0400 Received: from mail-io0-f182.google.com ([209.85.223.182]:35452 "EHLO mail-io0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934128AbdGSTpW (ORCPT ); Wed, 19 Jul 2017 15:45:22 -0400 MIME-Version: 1.0 In-Reply-To: <20170719193718.bvkkde5apbboudrk@treble> References: <1500422614-94821-1-git-send-email-keescook@chromium.org> <1500422614-94821-3-git-send-email-keescook@chromium.org> <20170719193718.bvkkde5apbboudrk@treble> From: Kees Cook Date: Wed, 19 Jul 2017 12:45:19 -0700 X-Google-Sender-Auth: wrNJbD4MYhdrQ0MYScXbZ7EIebk Message-ID: Subject: Re: [PATCH v6 2/2] x86/refcount: Implement fast refcount overflow protection To: Josh Poimboeuf Cc: Ingo Molnar , Peter Zijlstra , Christoph Hellwig , "Eric W. Biederman" , Andrew Morton , Jann Horn , Eric Biggers , Elena Reshetova , Hans Liljestrand , Greg KH , Alexey Dobriyan , "Serge E. Hallyn" , arozansk@redhat.com, Davidlohr Bueso , Manfred Spraul , "axboe@kernel.dk" , James Bottomley , "x86@kernel.org" , Arnd Bergmann , "David S. Miller" , Rik van Riel , LKML , linux-arch , "kernel-hardening@lists.openwall.com" 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 Wed, Jul 19, 2017 at 12:37 PM, Josh Poimboeuf wrote: > On Tue, Jul 18, 2017 at 05:03:34PM -0700, Kees Cook wrote: >> +/* >> + * Body of refcount error handling: in .text.unlikely, saved into CX the >> + * address of the refcount that has entered a bad state, and trigger an >> + * exception. Fixup address is back in regular execution flow in .text. >> + */ >> +#define _REFCOUNT_EXCEPTION \ >> + ".pushsection .text.unlikely\n" \ >> + "111:\tlea %[counter], %%" _ASM_CX "\n" \ >> + "112:\t" ASM_UD0 "\n" \ >> + ".popsection\n" \ >> + "113:\n" \ >> + _ASM_EXTABLE_REFCOUNT(112b, 113b) > > This confuses the freshly merged objtool 2.0, which is now too smart for > its own good. It's reporting some errors like: > > >> kernel/sched/autogroup.o: warning: objtool: sched_autogroup_exit()+0x48: return with modified stack frame > >> kernel/sched/autogroup.o: warning: objtool: .text.unlikely+0x27: stack state mismatch: reg1[3]=-2-40 reg2[3]=-2-24 > >> kernel/sched/autogroup.o: warning: objtool: sched_autogroup_exit()+0x14: stack state mismatch: reg1[3]=-2-40 reg2[3]=-2-24 > > Because the UD instructions are used for both WARN and BUG, objtool > doesn't know whether control flow continues past the instruction. So in > cases like this, it needs an "unreachable" annotation. > > Here's a patch to fix it, feel free to squash it into yours: Thanks! I'll add it for v7. > diff --git a/arch/x86/include/asm/refcount.h b/arch/x86/include/asm/refcount.h > index 13b91e850a02..e7587db3487c 100644 > --- a/arch/x86/include/asm/refcount.h > +++ b/arch/x86/include/asm/refcount.h > @@ -15,6 +15,7 @@ > ".pushsection .text.unlikely\n" \ > "111:\tlea %[counter], %%" _ASM_CX "\n" \ > "112:\t" ASM_UD0 "\n" \ > + ASM_UNREACHABLE \ > ".popsection\n" \ > "113:\n" \ > _ASM_EXTABLE_REFCOUNT(112b, 113b) > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index cd4bbe8242bd..85e0b8f42ca0 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -202,15 +202,25 @@ > #endif > > #ifdef CONFIG_STACK_VALIDATION > + > #define annotate_unreachable() ({ \ > asm("%c0:\t\n" \ > - ".pushsection .discard.unreachable\t\n" \ > - ".long %c0b - .\t\n" \ > - ".popsection\t\n" : : "i" (__LINE__)); \ > + ".pushsection .discard.unreachable\n\t" \ > + ".long %c0b - .\n\t" \ > + ".popsection\n\t" : : "i" (__LINE__)); \ Is this just an indentation change? > }) > + > +#define ASM_UNREACHABLE \ > + "999: .pushsection .discard.unreachable\n\t" \ > + ".long 999b - .\n\t" \ > + ".popsection\n\t" Just so I understand, we'll get a single byte added for each exception case, but it'll get discarded during final link? > + > #else > + > #define annotate_unreachable() > -#endif > +#define ASM_UNREACHABLE > + > +#endif /* CONFIG_STACK_VALIDATION */ > > /* > * Mark a position in code as unreachable. This can be used to Thanks! -Kees -- Kees Cook Pixel Security