linux-kernel - Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch

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]

Message-ID: <20150718134415.GA24587@treble.redhat.com>
Date:	Sat, 18 Jul 2015 08:44:15 -0500
From:	Josh Poimboeuf <jpoimboe@...hat.com>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, Michal Marek <mmarek@...e.cz>,
	Peter Zijlstra <peterz@...radead.org>,
	Andy Lutomirski <luto@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andi Kleen <andi@...stfloor.org>,
	Pedro Alves <palves@...hat.com>, x86@...nel.org,
	live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for
 __arch_hweight*()

On Sat, Jul 18, 2015 at 07:05:36AM +0200, Borislav Petkov wrote:
> On Fri, Jul 17, 2015 at 12:32:20PM -0500, Josh Poimboeuf wrote:
> > Well, but this isn't some whitelist code to make stackvalidate happy.
> > 
> > It's actually a real runtime frame pointer bug, and the rsp dependency
> > is real.  If it does the call without first creating the stack frame
> > then it breaks frame pointer based stack traces.
> 
> I think we can live with the stack trace being a little wrong in those
> __sw_* variants. And besides, we're talking about the very very small
> percentage of machines (which keeps getting smaller) which don't
> support POPCNT. And from those, only for the cases where the arg is not
> __builtin_constant_p() because there we do the __const_hweight* thing.
> 
> I'd prefer to not clutter the code more in that case.

Ok, so would you rather adding a whitelist to tell stackvalidate to
ignore it?  Something like this?

diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
index 9686c3d..d604691 100644
--- a/arch/x86/include/asm/arch_hweight.h
+++ b/arch/x86/include/asm/arch_hweight.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_X86_HWEIGHT_H
 #define _ASM_X86_HWEIGHT_H
 
+#include <linux/stackvalidate.h>
+
 #ifdef CONFIG_64BIT
 /* popcnt %edi, %eax -- redundant REX prefix for alignment */
 #define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc7"
@@ -25,7 +27,9 @@ static inline unsigned int __arch_hweight32(unsigned int w)
 {
 	unsigned int res = 0;
 
-	asm (ALTERNATIVE("call __sw_hweight32", POPCNT32, X86_FEATURE_POPCNT)
+	asm (ALTERNATIVE(STACKVALIDATE_IGNORE_INSN
+			 "call __sw_hweight32",
+			 POPCNT32, X86_FEATURE_POPCNT)
 		     : "="REG_OUT (res)
 		     : REG_IN (w));
 
@@ -50,7 +54,9 @@ static inline unsigned long __arch_hweight64(__u64 w)
 	return  __arch_hweight32((u32)w) +
 		__arch_hweight32((u32)(w >> 32));
 #else
-	asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT)
+	asm (ALTERNATIVE(STACKVALIDATE_IGNORE_INSN
+			 "call __sw_hweight64",
+			 POPCNT64, X86_FEATURE_POPCNT)
 		     : "="REG_OUT (res)
 		     : REG_IN (w));
 #endif /* CONFIG_X86_32 */

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/