[PATCH] x86, nospec: Fix uninitialized mask in array_index_mask_nospec()

[PATCH] x86, nospec: Fix uninitialized mask in array_index_mask_nospec()

[Dan Williams]

Break the false dependency on the uninitialized value of 'mask' in
array_index_mask_nospec(). This potentially allows the CPU to rename the
register, avoids reports from tools that can detect uninitialized inline
asm arguments, and makes it explicit that this sequence wants the result
of (0 - 0 - CARRY).

Reported-by: Andi Kleen <ak@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/barrier.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index e1259f043ae9..b572b6a77e12 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -36,10 +36,10 @@
 static inline unsigned long array_index_mask_nospec(unsigned long index,
 		unsigned long size)
 {
-	unsigned long mask;
+	unsigned long mask = 0;
 
-	asm ("cmp %1,%2; sbb %0,%0;"
-			:"=r" (mask)
+	asm ("cmp %1,%2; sbb $0,%0;"
+			:"+r" (mask)
 			:"g"(size),"r" (index)
 			:"cc");
 	return mask;

Re: [PATCH] x86, nospec: Fix uninitialized mask in array_index_mask_nospec()

[Linus Torvalds]

On Wed, Feb 14, 2018 at 12:50 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> Break the false dependency on the uninitialized value of 'mask' in
> array_index_mask_nospec(). This potentially allows the CPU to rename the
> register, avoids reports from tools that can detect uninitialized inline
> asm arguments, and makes it explicit that this sequence wants the result
> of (0 - 0 - CARRY).

NAK.

This is stupid.

Any asm tools that think this is "uninitialized inline asm arguments"
are complete garbage. It's an output-only register. Fix the tool, or
ignore it as shit.

And that false dependency argument is pure garbage too. Making this an
input that has been initialized to zero doesn't just generate a
pointless zeroing operation, it also adds to register pressure.

So NAK NAK NAK. The arguments for this patch are pure and utter shite.

If somebody can actually show a performance reason (with NUMBERS!) for
it or some _real_ reason to do something this stupid, that's one
thing. But these arguments are just bad, and the patch is bad.

              Linus