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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Fri, 14 Nov 2014 12:10:15 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Jay Vosburgh <jay.vosburgh@...onical.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	discuss@...nvswitch.org, pshelar@...ira.com, ogerlitz@...lanox.com
Subject: Re: net-next panic in ovs call to arch_fast_hash2 since e5a2c899

Hi Jay,

On Do, 2014-11-13 at 21:04 -0800, Jay Vosburgh wrote:
> 	[ adding Hannes to Cc, which I should've done initially ]
> 
> David Miller <davem@...emloft.net> wrote:
> 
> >From: Jay Vosburgh <jay.vosburgh@...onical.com>
> >Date: Thu, 13 Nov 2014 18:15:32 -0800
> >
> >> 	The "have feature" function, __intel_crc4_2_hash2, does not
> >> clobber %r8, and so the call does not panic on a system with
> >> X86_FEATURE_XMM4_2, although I'm not sure if that's a deliberate
> >> compiler action or just happenstance because __intel_crc4_2_hash2 uses
> >> fewer registers than __jhash2.
> >
> >Perhaps alternative calls can only be used with assembler routines
> >that use specific calling conventions, and they therefore generally
> >don't work with C functions?
> 
> 	I don't know the answer to that, but a quick search suggests
> that arch_fast_hash and arch_fast_hash2 (both added by commit e5a2c899)
> may be the only cases of alternative calls that aren't supplying either
> single instructions or assembly language functions.
> 
> 	From looking at how the alternative calls are implemented (code
> patching at boot or module load time from a table stored in a special
> section of the object file), I'm skeptical that the compiler could know
> what's the right thing to do.
> 
> 	Hannes, can you shed any light on this?

Unfortunately I only tested this code with rhashtable, which itself
takes a function pointer to arch_fast_hash and thus doesn't need to care
about clobbering so much. As a first step, I am currently testing this
patch as a first step and check thoroughly. Thanks for the report:

diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..1b32c82 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -4,45 +4,12 @@
 #include <linux/cpufeature.h>
 #include <asm/alternative.h>
 
-u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed);
-u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed);
-
-/*
- * non-inline versions of jhash so gcc does not need to generate
- * duplicate code in every object file
- */
-u32 __jhash(const void *data, u32 len, u32 seed);
-u32 __jhash2(const u32 *data, u32 len, u32 seed);
-
 /*
  * for documentation of these functions please look into
  * <include/asm-generic/hash.h>
  */
 
-static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
-{
-       u32 hash;
-
-       alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
-                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
-                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
-       return hash;
-}
-
-static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
-{
-       u32 hash;
-
-       alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
-                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
-                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
-       return hash;
-}
+u32 arch_fast_hash(const void *data, u32 len, u32 seed);
+u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed);
 
 #endif /* __ASM_X86_HASH_H */
diff --git a/arch/x86/lib/hash.c b/arch/x86/lib/hash.c
index e143271..a0a7a7e 100644
--- a/arch/x86/lib/hash.c
+++ b/arch/x86/lib/hash.c
@@ -38,7 +38,7 @@
 #include <linux/hash.h>
 #include <linux/jhash.h>
 
-static inline u32 crc32_u32(u32 crc, u32 val)
+static u32 crc32_u32(u32 crc, u32 val)
 {
 #ifdef CONFIG_AS_CRC32
        asm ("crc32l %1,%0\n" : "+r" (crc) : "rm" (val));
@@ -71,7 +71,6 @@ u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed)
 
        return seed;
 }
-EXPORT_SYMBOL(__intel_crc4_2_hash);
 
 u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
 {
@@ -82,16 +81,45 @@ u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
 
        return seed;
 }
-EXPORT_SYMBOL(__intel_crc4_2_hash2);
 
 u32 __jhash(const void *data, u32 len, u32 seed)
 {
        return jhash(data, len, seed);
 }
-EXPORT_SYMBOL(__jhash);
 
 u32 __jhash2(const u32 *data, u32 len, u32 seed)
 {
        return jhash2(data, len, seed);
 }
-EXPORT_SYMBOL(__jhash2);
+
+noinline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
+{
+       u32 hash;
+
+
+#ifdef CONFIG_X86_64
+       alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
+#else
+       alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
+#endif
+       return hash;
+}
+EXPORT_SYMBOL(arch_fast_hash);
+
+noinline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
+{
+       u32 hash;
+
+
+#ifdef CONFIG_X86_64
+       alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
+#else
+       alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
+#endif
+       return hash;
+}
+EXPORT_SYMBOL(arch_fast_hash2);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists