linux-kernel - Re: [PATCH v6 2/2] x86/refcount: Implement fast refcount overflow protection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 Jul 2017 14:37:18 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Christoph Hellwig <hch@...radead.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jann Horn <jannh@...gle.com>,
        Eric Biggers <ebiggers3@...il.com>,
        Elena Reshetova <elena.reshetova@...el.com>,
        Hans Liljestrand <ishkamiel@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        "Serge E. Hallyn" <serge@...lyn.com>, arozansk@...hat.com,
        Davidlohr Bueso <dave@...olabs.net>,
        Manfred Spraul <manfred@...orfullife.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        "x86@...nel.org" <x86@...nel.org>, Arnd Bergmann <arnd@...db.de>,
        "David S. Miller" <davem@...emloft.net>,
        Rik van Riel <riel@...hat.com>, linux-kernel@...r.kernel.org,
        linux-arch <linux-arch@...r.kernel.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v6 2/2] x86/refcount: Implement fast refcount overflow
 protection

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:


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__));			\
 })
+
+#define ASM_UNREACHABLE							\
+	"999: .pushsection .discard.unreachable\n\t"			\
+	".long 999b - .\n\t"						\
+	".popsection\n\t"
+
 #else
+
 #define annotate_unreachable()
-#endif
+#define ASM_UNREACHABLE
+
+#endif /* CONFIG_STACK_VALIDATION */
 
 /*
  * Mark a position in code as unreachable.  This can be used to