On these architectures, struct gu_rse (defined at galerautils/src/gu_rand.c:19) has holes in it due to alignment. In function gu_rand_seed_long(galerautils/src/gu_rand.c:30), a struct of this type is initialised on a by-member basis. But its address is then passed to gu_fast_hash64_medium which reads it as a buffer. So any holes in that buffer remain uninitialized.
I presume that powerpc and armhf are the architectures where struct gu_rse end up with holes in it, and the other architectures do not. However, gcc is free to change this in the future, so I think this bug applies to all architectures, and a proper fix would not be architecture-specific.
Applying the following patch fixes the issue and so is consistent with my hypothesis:
/*! Structure to hold entropy data.
* Should be at least 20 bytes on 32-bit systems and 28 bytes on 64-bit */
-struct gu_rse
+struct __attribute__((__packed__)) gu_rse
{
long long time;
const void* heap_ptr;
It would seem cleaner to me to memset(&rse, 0, sizeof(rse)) though. gcc would probably optimise the initialization fully, leaving architectures without the holes unchanged.
Can I have some feedback from upstream on this though please? Is this code being used for anything that needs to be cryptographically secure? Will you accept a memset fix, and will this have any unintended side-effects?
My hypothesis:
On these architectures, struct gu_rse (defined at galerautils/ src/gu_ rand.c: 19) has holes in it due to alignment. In function gu_rand_ seed_long( galerautils/ src/gu_ rand.c: 30), a struct of this type is initialised on a by-member basis. But its address is then passed to gu_fast_ hash64_ medium which reads it as a buffer. So any holes in that buffer remain uninitialized.
I presume that powerpc and armhf are the architectures where struct gu_rse end up with holes in it, and the other architectures do not. However, gcc is free to change this in the future, so I think this bug applies to all architectures, and a proper fix would not be architecture- specific.
Applying the following patch fixes the issue and so is consistent with my hypothesis:
--- percona- galera- 3-3.8-3390. orig/galerautil s/src/gu_ rand.c galera- 3-3.8-3390/ galerautils/ src/gu_ rand.c
+++ percona-
@@ -16,7 +16,7 @@
/*! Structure to hold entropy data. _((__packed_ _)) gu_rse
* Should be at least 20 bytes on 32-bit systems and 28 bytes on 64-bit */
-struct gu_rse
+struct __attribute_
{
long long time;
const void* heap_ptr;
It would seem cleaner to me to memset(&rse, 0, sizeof(rse)) though. gcc would probably optimise the initialization fully, leaving architectures without the holes unchanged.
Can I have some feedback from upstream on this though please? Is this code being used for anything that needs to be cryptographically secure? Will you accept a memset fix, and will this have any unintended side-effects?