Add a pointer alignment helper function. Also use a slightly more conservative pattern. Instead of aligning the pointer as a uintptr_t and casting back, compute the offset and advance in pointer space. C guarantees that casting from pointer to uintptr_t and back gives the same pointer, but general integer-to-pointer conversions are generally implementation-defined. GCC does define it in the useful way, but this makes fewer dependencies. Change-Id: I70c7af735e892fe7a8333b78b39d7b1f3f1cdbef Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48405 Reviewed-by: Adam Langley <alangley@gmail.com>
diff --git a/crypto/hrss/hrss.c b/crypto/hrss/hrss.c index 67ff4c1..957e9d2 100644 --- a/crypto/hrss/hrss.c +++ b/crypto/hrss/hrss.c
@@ -1871,9 +1871,7 @@ sizeof(struct HRSS_public_key) >= sizeof(struct public_key) + 15, "HRSS public key too small"); - uintptr_t p = (uintptr_t)ext; - p = (p + 15) & ~15; - return (struct public_key *)p; + return align_pointer(ext->opaque, 16); } // private_key_from_external does the same thing as |public_key_from_external|, @@ -1885,9 +1883,7 @@ sizeof(struct HRSS_private_key) >= sizeof(struct private_key) + 15, "HRSS private key too small"); - uintptr_t p = (uintptr_t)ext; - p = (p + 15) & ~15; - return (struct private_key *)p; + return align_pointer(ext->opaque, 16); } void HRSS_generate_key(
diff --git a/crypto/internal.h b/crypto/internal.h index 14c60e09..b288583 100644 --- a/crypto/internal.h +++ b/crypto/internal.h
@@ -209,6 +209,9 @@ #define OPENSSL_SSE2 #endif + +// Pointer utility functions. + // buffers_alias returns one if |a| and |b| alias and zero otherwise. static inline int buffers_alias(const uint8_t *a, size_t a_len, const uint8_t *b, size_t b_len) { @@ -221,6 +224,23 @@ return a_u + a_len > b_u && b_u + b_len > a_u; } +// align_pointer returns |ptr|, advanced to |alignment|. |alignment| must be a +// power of two, and |ptr| must have at least |alignment - 1| bytes of scratch +// space. +static inline void *align_pointer(void *ptr, size_t alignment) { + // |alignment| must be a power of two. + assert(alignment != 0 && (alignment & (alignment - 1)) == 0); + // Instead of aligning |ptr| as a |uintptr_t| and casting back, compute the + // offset and advance in pointer space. C guarantees that casting from pointer + // to |uintptr_t| and back gives the same pointer, but general + // integer-to-pointer conversions are implementation-defined. GCC does define + // it in the useful way, but this makes fewer assumptions. + uintptr_t offset = (0u - (uintptr_t)ptr) & (alignment - 1); + ptr = (char *)ptr + offset; + assert(((uintptr_t)ptr & (alignment - 1)) == 0); + return ptr; +} + // Constant-time utility functions. //
diff --git a/crypto/poly1305/poly1305.c b/crypto/poly1305/poly1305.c index 31a567d..c07b1e9 100644 --- a/crypto/poly1305/poly1305.c +++ b/crypto/poly1305/poly1305.c
@@ -56,7 +56,7 @@ static inline struct poly1305_state_st *poly1305_aligned_state( poly1305_state *state) { - return (struct poly1305_state_st *)(((uintptr_t)state + 63) & ~63); + return align_pointer(state, 64); } // poly1305_blocks updates |state| given some amount of input data. This
diff --git a/tool/speed.cc b/tool/speed.cc index 1b89b42..264334f 100644 --- a/tool/speed.cc +++ b/tool/speed.cc
@@ -342,12 +342,6 @@ return true; } -static uint8_t *align(uint8_t *in, unsigned alignment) { - return reinterpret_cast<uint8_t *>( - (reinterpret_cast<uintptr_t>(in) + alignment) & - ~static_cast<size_t>(alignment - 1)); -} - static std::string ChunkLenSuffix(size_t chunk_len) { char buf[32]; snprintf(buf, sizeof(buf), " (%zu byte%s)", chunk_len, @@ -384,13 +378,17 @@ new uint8_t[overhead_len + kAlignment]); - uint8_t *const in = align(in_storage.get(), kAlignment); + uint8_t *const in = + static_cast<uint8_t *>(align_pointer(in_storage.get(), kAlignment)); OPENSSL_memset(in, 0, chunk_len); - uint8_t *const out = align(out_storage.get(), kAlignment); + uint8_t *const out = + static_cast<uint8_t *>(align_pointer(out_storage.get(), kAlignment)); OPENSSL_memset(out, 0, chunk_len + overhead_len); - uint8_t *const tag = align(tag_storage.get(), kAlignment); + uint8_t *const tag = + static_cast<uint8_t *>(align_pointer(tag_storage.get(), kAlignment)); OPENSSL_memset(tag, 0, overhead_len); - uint8_t *const in2 = align(in2_storage.get(), kAlignment); + uint8_t *const in2 = + static_cast<uint8_t *>(align_pointer(in2_storage.get(), kAlignment)); if (!EVP_AEAD_CTX_init_with_direction(ctx.get(), aead, key.get(), key_len, EVP_AEAD_DEFAULT_TAG_LENGTH,