Bug#35180841 Remove requirement for md5_hash() input to be 8-byte aligned

As described in bug report, there seems to be no valid reasons for
requiring the md5_hash input to be 8-byte aligned anymore.

It seems to just add complexity for the caller, and sometimes
an extra memcpy() may even be needed.

Patch replace the Uint64 accesses in md5_hash with memcpy() instead,
which seems to perform identical to the original DWORD assignments.
(And generate ~identical code as well)

md5_hash() signature is also changed to take a char* instead of
an Uint64*. Inlined variants of md5_hash, taking an Uint32* as
argument, is also provided for convenience and avoiding type casts
where md5_hash is normally called with a key stored in an Uint32[]

Usage of md5_hash is adapted accordingly.

Allocation of temporary buffers for md5_hash as Uint64[] has been
changed to Uint32[] as the Uint64 alignment requirements was removed.

Change-Id: I6089a3af1436d1e73620fabf01b32082519d9b38

Author: Ole John Aske

storage/ndb/include/util/md5_hash.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2023, Oracle and/or its affiliates.
     All rights reserved. Use is subject to license terms.
 
    This program is free software; you can redistribute it and/or modify
@@ -28,15 +28,33 @@
 
 #include <ndb_types.h>
 
-// External declaration of hash function 
-void md5_hash(Uint32 result[4], const Uint64* keybuf, Uint32 no_of_32_words);
+/**
+ * Calculate a md5-hash of keyBuf into result. If 'no_of_bytes' in keybuf
+ * is not an 32 bit aligned word, the hash is calculated as if keybuf
+ * were zero-padded to the upper word aligned length.
+ * Note that there is no alignment requirement on the keybuf itself.
+ */
+void md5_hash(Uint32 result[4],
+              const char* keybuf,
+              Uint32 no_of_bytes);
+
+
+// Convenient variants as keys are often stored in an Uint32 array
+inline
+void md5_hash(Uint32 result[4],
+              const Uint32* keybuf,
+              Uint32 no_of_words)
+{
+  md5_hash(result, reinterpret_cast<const char*>(keybuf), no_of_words*4);
+}
 
 inline
 Uint32
-md5_hash(const Uint64* keybuf, Uint32 no_of_32_words)
+md5_hash(const Uint32* keybuf,
+         Uint32 no_of_words)
 {
   Uint32 result[4];
-  md5_hash(result, keybuf, no_of_32_words);
+  md5_hash(result, reinterpret_cast<const char*>(keybuf), no_of_words*4);
   return result[0];
 }
 

storage/ndb/src/common/util/md5_hash.cpp

@@ -27,9 +27,10 @@
 
 #include "md5_hash.hpp"
 
-#ifdef WORDS_BIGENDIAN
-#define HIGHFIRST 1
-#endif
+#include <string.h>       // memcpy()
+#include <my_compiler.h>  // likely
+#include <my_config.h>    // big/little endian
+
 
 /*
  * This code implements the MD5 message-digest algorithm.
@@ -48,25 +49,6 @@
  * is returned as the hash value.
  */
 
-#ifndef HIGHFIRST
-#define byteReverse(buf, len)	/* Nothing */
-#else
-void byteReverse(unsigned char *buf, unsigned longs);
-/*
- * Note: this code is harmless on little-endian machines.
- */
-void byteReverse(unsigned char *buf, unsigned longs)
-{
-    Uint32 t;
-    do {
-	t = (Uint32) ((unsigned) buf[3] << 8 | buf[2]) << 16 |
-	    ((unsigned) buf[1] << 8 | buf[0]);
-	*(Uint32 *) buf = t;
-	buf += 4;
-    } while (--longs);
-}
-#endif
-
 /* The four core functions - F1 is optimized somewhat */
 
 /* #define F1(x, y, z) (x & y | ~x & z) */
@@ -168,47 +150,46 @@ static void MD5Transform(Uint32 buf[4], Uint32 const in[16])
 }
 
 /*
- * Start MD5 accumulation.  Set bit count to 0 and buffer to mysterious
+ * Start MD5 accumulation. Set bit count to 0 and buffer to mysterious
  * initialization constants.
  */
-void md5_hash(Uint32 result[4], const Uint64* keybuf, Uint32 no_of_32_words)
+void md5_hash(Uint32 result[4],
+              const char* keybuf,
+              Uint32 no_of_bytes)
 {
-  /**
-   * This is the external interface of the module
-   * It is assumed that keybuf is placed on 8 byte
-   * alignment.
-   */
-  Uint32 i;
   Uint32 buf[4];
   union {
     Uint64 transform64_buf[8];
     Uint32 transform32_buf[16];
   };
-  Uint32 len = no_of_32_words << 2;
-  const Uint64* key64buf = (const Uint64*)keybuf;
-  const Uint32* key32buf = (const Uint32*)keybuf;
+
+  // Round up to full word length
+  const Uint32 len = (no_of_bytes + 3) & ~3;
 
   buf[0] = 0x67452301;
   buf[1] = 0xefcdab89;
   buf[2] = 0x98badcfe;
   buf[3] = 0x10325476;
 
-  while (no_of_32_words >= 16) {
-    transform64_buf[0] = key64buf[0];
-    transform64_buf[1] = key64buf[1];
-    transform64_buf[2] = key64buf[2];
-    transform64_buf[3] = key64buf[3];
-    transform64_buf[4] = key64buf[4];
-    transform64_buf[5] = key64buf[5];
-    transform64_buf[6] = key64buf[6];
-    transform64_buf[7] = key64buf[7];
-    no_of_32_words -= 16;
-    key64buf += 8;
-    byteReverse((unsigned char *)transform32_buf, 16);
+  while (no_of_bytes >= 64) {
+    memcpy(transform32_buf, keybuf, 64);
+    keybuf += 64;
+    no_of_bytes -= 64;
+    MD5Transform(buf, transform32_buf);
+  }
+
+  if (unlikely(no_of_bytes >= 61)) { // Will be a full frame when padded
+    // Last word written will not be a full word -> zero pad before memcpy over
+    transform32_buf[15] = 0;
+    memcpy(transform32_buf, keybuf, no_of_bytes);
+    keybuf += no_of_bytes;
+    no_of_bytes = 0;
     MD5Transform(buf, transform32_buf);
   }
 
-  key32buf = (const Uint32*)key64buf;
+  // Remaining words, including zero padding, to get to a word-aligned size.
+  const Uint32 no_of_32_words = (no_of_bytes + 3) >> 2;
+
   transform64_buf[0] = 0;
   transform64_buf[1] = 0;
   transform64_buf[2] = 0;
@@ -218,35 +199,47 @@ void md5_hash(Uint32 result[4], const Uint64* keybuf, Uint32 no_of_32_words)
   transform64_buf[6] = 0;
   transform64_buf[7] = (Uint64)len;
 
-  for (i = 0; i < no_of_32_words; i++)
-    transform32_buf[i] = key32buf[i];
+  // 0x800... Is used as an end / length mark, possibly overwriting the 'len'.
   transform32_buf[no_of_32_words] = 0x80000000;
 
-  if (no_of_32_words < 14) {
-    byteReverse((unsigned char *)transform32_buf, 16);
-    MD5Transform(buf, transform32_buf);
-  } else {
-    if (no_of_32_words == 14)
-      transform32_buf[15] = 0;
-    MD5Transform(buf, transform32_buf);
-    transform64_buf[0] = 0;
-    transform64_buf[1] = 0;
-    transform64_buf[2] = 0;
-    transform64_buf[3] = 0;
-    transform64_buf[4] = 0;
-    transform64_buf[5] = 0;
-    transform64_buf[6] = 0;
-    transform64_buf[7] = (Uint64)len;
-    byteReverse((unsigned char *)transform32_buf, 16);
-    MD5Transform(buf, transform32_buf);    
+  if (likely(no_of_32_words > 0)) {
+    // Last word written may not be a full word -> zero pad before memcpy over
+    transform32_buf[no_of_32_words-1] = 0;
+    memcpy(transform32_buf, keybuf, no_of_bytes);
+
+    if (unlikely(no_of_32_words >= 14)) {
+      // On a little endian platform the memcpy() + 0x800.. wrote over 'len'
+      // located at 32_buf[14], 32_buf[15] was already zero.
+      // On a big endian, len is written to 32_buf[15] and not written over
+      // if '#32_words == 14' -> We clear it as it is set in next frame instead.
+#ifdef WORDS_BIGENDIAN
+      if (no_of_32_words == 14)
+        transform32_buf[15] = 0;
+#endif
+      // Note: 'len' should have been written to 32_buf[15] for both
+      //       big/little endian.
+      // For backward bug compatability it is too late to fix it now.
+      MD5Transform(buf, transform32_buf);
+      transform64_buf[0] = 0;
+      transform64_buf[1] = 0;
+      transform64_buf[2] = 0;
+      transform64_buf[3] = 0;
+      transform64_buf[4] = 0;
+      transform64_buf[5] = 0;
+      transform64_buf[6] = 0;
+      transform64_buf[7] = (Uint64)len;
+    }
   }
+  MD5Transform(buf, transform32_buf);
 
   result[0] = buf[0];
   result[1] = buf[1];
   result[2] = buf[2];
   result[3] = buf[3];
 }
 
+
+
 //////////////////////////////////////////////////
 //////////////////////////////////////////////////
 //////////////////////////////////////////////////
@@ -371,27 +364,6 @@ test_sample test_samples[] = {
 };
 #endif
 
-
-/**
- * Glue function for allowing to test non-word-aligned 'no_of_bytes'.
- * Will zero-pad the 'key' to a word aligned length before hashing. 
- */
-void md5_hash(Uint32 result[4],
-              const char* keybuf,
-              Uint32 no_of_bytes)
-{
-  union {
-    char tmp[MAX_TEST_SAMPLE_WORDS*4];
-    Uint64 _align;
-  };
-  memcpy(tmp, keybuf, no_of_bytes);
-  while (no_of_bytes & 3) {
-    tmp[no_of_bytes++] = 0;
-  }
-  md5_hash(result, (Uint64*)tmp, no_of_bytes / 4);
-}
-
-
 int main()
 {
   Uint32 test_data[MAX_TEST_SAMPLE_WORDS];

storage/ndb/src/kernel/blocks/dbacc/Dbacc.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2022, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -1202,11 +1202,7 @@ struct Tabrec {
   BlockReference cndbcntrRef;
   Uint16 csignalkey;
   Uint32 czero;
-  union {
   Uint32 ckeys[2048 * MAX_XFRM_MULTIPLY];
-  Uint64 ckeys_align;
-  };
-  
   Uint32 c_errorInsert3000_TableId;
   Uint32 c_memusage_report_frequency;
 };

storage/ndb/src/kernel/blocks/dbacc/DbaccMain.cpp

@@ -1,4 +1,4 @@
-/* Copyright (c) 2003, 2022, Oracle and/or its affiliates.
+/* Copyright (c) 2003, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -5846,7 +5846,7 @@ LHBits32 Dbacc::getElementHash(OperationrecPtr& oprec)
     localkey = oprec.p->localdata;
     Uint32 len = readTablePk(localkey.m_page_no, localkey.m_page_idx, ElementHeader::setLocked(oprec.i), oprec);
     if (len > 0)
-      oprec.p->hashValue = LHBits32(md5_hash((Uint64*)ckeys, len));
+      oprec.p->hashValue = LHBits32(md5_hash(ckeys, len));
   }
   return oprec.p->hashValue;
 }
@@ -5868,7 +5868,7 @@ LHBits32 Dbacc::getElementHash(Uint32 const* elemptr)
   if (len > 0)
   {
     jam();
-    return LHBits32(md5_hash((Uint64*)ckeys, len));
+    return LHBits32(md5_hash(ckeys, len));
   }
   else
   { // Return an invalid hash value if no data

storage/ndb/src/kernel/blocks/dblqh/DblqhMain.cpp

@@ -6221,7 +6221,7 @@ Dblqh::nr_copy_delete_row(Signal* signal,
     }
     else
     {
-      req->hashValue = md5_hash((Uint64*)(signal->theData+24), len);
+      req->hashValue = md5_hash(signal->theData+24, len);
     }
     req->keyLen = 0; // seach by local key
     req->localKey[0] = rowid->m_page_no;
@@ -14094,13 +14094,13 @@ Uint32
 Dblqh::calculateHash(Uint32 tableId, const Uint32* src) 
 {
   jam();
-  Uint64 Tmp[(MAX_KEY_SIZE_IN_WORDS*MAX_XFRM_MULTIPLY) >> 1];
+  Uint32 tmp[MAX_KEY_SIZE_IN_WORDS*MAX_XFRM_MULTIPLY];
   Uint32 keyPartLen[MAX_ATTRIBUTES_IN_INDEX];
-  Uint32 keyLen = xfrm_key(tableId, src, (Uint32*)Tmp, sizeof(Tmp) >> 2, 
+  Uint32 keyLen = xfrm_key(tableId, src, tmp, sizeof(tmp) >> 2,
 			   keyPartLen);
   ndbrequire(keyLen);
-  
-  return md5_hash(Tmp, keyLen);
+
+  return md5_hash(tmp, keyLen);
 }//Dblqh::calculateHash()
 
 /**
@@ -14806,7 +14806,7 @@ void Dblqh::copyTupkeyConfLab(Signal* signal)
   }
   else
   {
-    tcConnectptr.p->hashValue = md5_hash((Uint64*)tmp, len);
+    tcConnectptr.p->hashValue = md5_hash(tmp, len);
   }
 
   // Copy keyinfo into long section for LQHKEYREQ below

storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2012, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2012, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -1529,7 +1529,7 @@ class Dbspj: public SimulatedBlock {
   void lookup_cleanup(Ptr<Request>, Ptr<TreeNode>);
 
   Uint32 handle_special_hash(Uint32 tableId, Uint32 dstHash[4],
-                             const Uint64* src,
+                             const Uint32* src,
                              Uint32 srcLen,       // Len in #32bit words
                              const struct KeyDescriptor* desc);