Fix of crash in e_rc4_hmac_md5.c caused by rc4-md5-x86_64.S.

openssl.gyp

Index: openssl.gyp
diff --git a/openssl.gyp b/openssl.gyp
index 1b5430d7635a3ac077eaa254545c808d817ef066..d3bdbcaf1b2804e88ab66d7553343eac78e17645 100644
--- a/openssl.gyp
+++ b/openssl.gyp
@@ -65,20 +65,10 @@
         ['target_arch == "x64"', {
           'sources': [ '<@(openssl_x86_64_sources)' ],
           'sources!': [ '<@(openssl_x86_64_source_excludes)' ],
-          'conditions': [
-            ['OS != "android"', {
-              # Because rc4-x86_64.S has a problem,
-              # We use the C rc4 source instead of the ASM source.
-              # This hurts performance, but it's not a problem
-              # because no production code uses openssl on x86-64.
-              'sources/': [
-                ['exclude', 'openssl/crypto/rc4/asm/rc4-x86_64\\.S' ],
-                ['include', 'openssl/crypto/rc4/rc4_enc\\.c' ],
-                ['include', 'openssl/crypto/rc4/rc4_skey\\.c' ],
-              ],
-            }]
+          'defines': [
+            '<@(openssl_x86_64_defines)',
+            'RC4_INT=unsigned int'

[wtc, 2014/02/04 18:20:19]

Summary: It seems better to change the RC4_INT definition in
third_party/openssl/config/x64/openssl/opensslconf.h, but I don't know what's
the right way to generate that header.

Details:

I'm not familiar with this part of OpenSSL. When I run ./config in an upstream
openssl tree on Linux x86_64, the generated opensslconf.h header contains these
RC4-related macro definitions:

=====
#if defined(HEADER_RC4_H)
#if !defined(RC4_INT)
/* using int types make the structure larger but make the code faster
 * on most boxes I have tested - up to %20 faster. */
/*
 * I don't know what does "most" mean, but declaring "int" is a must on:
 * - Intel P6 because partial register stalls are very expensive;
 * - elder Alpha because it lacks byte load/store instructions;
 */
#define RC4_INT unsigned int
#endif
#if !defined(RC4_CHUNK)
/*
 * This enables code handling data aligned at natural CPU word
 * boundary. See crypto/rc4/rc4_enc.c for further details.
 */
#define RC4_CHUNK unsigned long
#endif
#endif

...

#if defined(HEADER_RC4_LOCL_H) && !defined(CONFIG_HEADER_RC4_LOCL_H)
#define CONFIG_HEADER_RC4_LOCL_H
/* if this is defined data[i] is used instead of *data, this is a %20
 * speedup on x86 */
#undef RC4_INDEX
#endif
=====

RC4-related assembly code is assembled with these command lines:

=====
/usr/bin/perl asm/rc4-x86_64.pl elf > rc4-x86_64.s
gcc -c -I.. -I../.. -I../modes -I../asn1 -I../evp -I../../include 
-DOPENSSL_THREADS -D_REENTRANT -DDSO_DLFCN -DHAVE_DLFCN_H -Wa,--noexecstack -m64
-DL_ENDIAN -DTERMIO -O3 -Wall -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT
-DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM
-DSHA512_ASM -DMD5_ASM -DAES_ASM -DVPAES_ASM -DBSAES_ASM -DWHIRLPOOL_ASM
-DGHASH_ASM -c  -o rc4-x86_64.o rc4-x86_64.s
/usr/bin/perl asm/rc4-md5-x86_64.pl elf > rc4-md5-x86_64.s
gcc -c -I.. -I../.. -I../modes -I../asn1 -I../evp -I../../include 
-DOPENSSL_THREADS -D_REENTRANT -DDSO_DLFCN -DHAVE_DLFCN_H -Wa,--noexecstack -m64
-DL_ENDIAN -DTERMIO -O3 -Wall -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT
-DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM
-DSHA512_ASM -DMD5_ASM -DAES_ASM -DVPAES_ASM -DBSAES_ASM -DWHIRLPOOL_ASM
-DGHASH_ASM -c  -o rc4-md5-x86_64.o rc4-md5-x86_64.s
=====

I found three copies of opensslconf.h in the Chromium source tree:

third_party/openssl/openssl/crypto/opensslconf.h
third_party/openssl/openssl/include/openssl/opensslconf.h
third_party/openssl/config/x64/openssl/opensslconf.h

The first two copies are the same. The third copy seems to be x86 specific but
only has minor differences from the first two copies:

=====
$ diff -pu third_party/openssl/openssl/include/openssl/opensslconf.h
third_party/openssl/config/x64/openssl/opensslconf.h
--- third_party/openssl/openssl/include/openssl/opensslconf.h	2013-06-20
15:19:02.562863938 -0700
+++ third_party/openssl/config/x64/openssl/opensslconf.h	2013-06-20
15:19:03.322875471 -0700
@@ -220,14 +220,14 @@
 
 #if defined(HEADER_BN_H) && !defined(CONFIG_HEADER_BN_H)
 #define CONFIG_HEADER_BN_H
-#define BN_LLONG
+#undef BN_LLONG
 
 /* Should we define BN_DIV2W here? */
 
 /* Only one for the following should be defined */
-#undef SIXTY_FOUR_BIT_LONG
+#define SIXTY_FOUR_BIT_LONG
 #undef SIXTY_FOUR_BIT
-#define THIRTY_TWO_BIT
+#undef THIRTY_TWO_BIT
 #endif
 
 #if defined(HEADER_RC4_LOCL_H) && !defined(CONFIG_HEADER_RC4_LOCL_H)
=====

If I diff the opensslconf.h generated by "./config" and the x64 opensslconf.h in
the Chromium source tree, I see these interesting differences:
=====
$ diff -pu ~/tmp/openssl/include/openssl/opensslconf.h
third_party/openssl/config/x64/openssl/opensslconf.h
---
/usr/local/google/home/wtc/tmp/openssl/include/openssl/opensslconf.h	2014-02-04
09:51:32.850733107 -0800
+++ third_party/openssl/config/x64/openssl/opensslconf.h	2013-06-20
15:19:03.322875471 -0700
@@ -129,7 +199,7 @@
  * - Intel P6 because partial register stalls are very expensive;
  * - elder Alpha because it lacks byte load/store instructions;
  */
-#define RC4_INT unsigned int
+#define RC4_INT unsigned char
 #endif
 #if !defined(RC4_CHUNK)
 /*
@@ -169,7 +239,7 @@
 
 #if defined(HEADER_BF_LOCL_H) && !defined(CONFIG_HEADER_BF_LOCL_H)
 #define CONFIG_HEADER_BF_LOCL_H
-#undef BF_PTR
+#define BF_PTR
 #endif /* HEADER_BF_LOCL_H */
 
 #if defined(HEADER_DES_LOCL_H) && !defined(CONFIG_HEADER_DES_LOCL_H)
=====

We should also look into why the x64 opensslconf.h in the Chromium source tree
defines the BF_PTR macro.

           ],
-          'defines': [ '<@(openssl_x86_64_defines)' ],
           'defines!': [ 'OPENSSL_NO_ASM' ],
           'variables': {
             # Ensure the 64-bit opensslconf.h header is used.