Comment 2 for bug 1430874

Revision history for this message
Robie Basak (racb) wrote :

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/galerautils/src/gu_rand.c
+++ percona-galera-3-3.8-3390/galerautils/src/gu_rand.c
@@ -16,7 +16,7 @@

 /*! 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?