Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()

[Josh Poimboeuf <jpoimboe@redhat.com>]

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