From f3d3cee4fedab2646d06603068b0dced68fdb49e Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 21 Apr 2017 17:23:11 -0400 Subject: [PATCH] Avoid messing with dummy functions in delocate.go. With some optimisation settings, Clang was loading BORINGSSL_bcm_text_hash with AVX2 instructions, which weren't getting translated correctly. This seems to work and is less fragile. The compiler just emits an leaq here. This is because it knows the symbol is hidden (in the shared library sense), so it needn't go through GOTPCREL. The assembler would have added a relocation, were the symbol left undefined, but since we define the symbol later on, it all works out without a relocation. Were the symbol not hidden, the compiler would have emitted a movq by way of GOTPCREL, but we can now translate those away anyway. Change-Id: I442a22f4f8afaadaacbab7044f946a963ebfc46c Reviewed-on: https://boringssl-review.googlesource.com/15384 Reviewed-by: Adam Langley --- crypto/fipsmodule/bcm.c | 18 ++++++++---------- crypto/fipsmodule/delocate.go | 4 ---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/crypto/fipsmodule/bcm.c b/crypto/fipsmodule/bcm.c index 7ba50a2c..b93ab834 100644 --- a/crypto/fipsmodule/bcm.c +++ b/crypto/fipsmodule/bcm.c @@ -77,12 +77,11 @@ static int check_test(const void *expected, const void *actual, #ifndef OPENSSL_ASAN -/* These functions are removed by delocate.go and references to them are - * rewritten to point to the start and end of the module, and the location of - * the integrity hash. */ -static void BORINGSSL_bcm_text_dummy_start(void) {} -static void BORINGSSL_bcm_text_dummy_end(void) {} -static void BORINGSSL_bcm_text_dummy_hash(void) {} +/* These symbols are filled in by delocate.go. They point to the start and end + * of the module, and the location of the integrity hash, respectively. */ +extern const uint8_t BORINGSSL_bcm_text_start[]; +extern const uint8_t BORINGSSL_bcm_text_end[]; +extern const uint8_t BORINGSSL_bcm_text_hash[]; #endif static void BORINGSSL_bcm_power_on_self_test(void) __attribute__((constructor)); @@ -91,8 +90,8 @@ static void BORINGSSL_bcm_power_on_self_test(void) { CRYPTO_library_init(); #ifndef OPENSSL_ASAN - const uint8_t *const start = (const uint8_t *)BORINGSSL_bcm_text_dummy_start; - const uint8_t *const end = (const uint8_t *)BORINGSSL_bcm_text_dummy_end; + const uint8_t *const start = BORINGSSL_bcm_text_start; + const uint8_t *const end = BORINGSSL_bcm_text_end; static const uint8_t kHMACKey[32] = {0}; uint8_t result[SHA256_DIGEST_LENGTH]; @@ -104,8 +103,7 @@ static void BORINGSSL_bcm_power_on_self_test(void) { goto err; } - const uint8_t *const expected = - (const uint8_t *)BORINGSSL_bcm_text_dummy_hash; + const uint8_t *expected = BORINGSSL_bcm_text_hash; if (!check_test(expected, result, sizeof(result), "FIPS integrity test")) { goto err; diff --git a/crypto/fipsmodule/delocate.go b/crypto/fipsmodule/delocate.go index 3de9ae38..2215f256 100644 --- a/crypto/fipsmodule/delocate.go +++ b/crypto/fipsmodule/delocate.go @@ -233,10 +233,6 @@ func transform(lines []string, symbols map[string]bool) (ret []string) { continue } - if parts[0] == "leaq" { - line = strings.Replace(line, "BORINGSSL_bcm_text_dummy_", "BORINGSSL_bcm_text_", -1) - } - target := strings.SplitN(parts[1], ",", 2)[0] if strings.HasSuffix(target, "(%rip)") { target = target[:len(target)-6]