Bug #29820838 NDB : LONG LQHKEYREQS CONTAIN INCORRECTLY SET BITS

In some situations, a number of bits are incorrectly set in the
long LQHKEYREQ requestInfo field.

This can cause misinterpretation on the LQH receiving the
LQHKEYREQ, which interprets them as feature switch booleans.

Changes :
  - The code which is incorrectly setting the bits is fixed so that
    they are not set.
    This fixes the bug for all future versions

  - A new self check is added on the affected bits which are currently
    unused
    They must not be set on long LQHKEYREQs arriving from same-version
    nodes.
    - They may be set on long LQHKEYREQs from :
      - Older version nodes, still having this bug
      - Newer version nodes, using the bits for new purposes

  - Note that for bits which are at-risk, there remains a risk of
    misinterpreting a set bit, until all versions are upgraded.

  - Related fix :
    The logic related to pass-through of requestInfo bits when
    forwarding LQHKEYREQs between replicas is also updated to
    pass through only the bits which are needed at Backup replicas.

The addition of the self check provides debug-build test coverage
existing testcases.

Reviewed by Mauritz Sundell <[email protected]>

Author: frazerclement

storage/ndb/include/kernel/signaldata/LqhKey.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+   Copyright (c) 2003, 2019 Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -183,6 +183,11 @@ class LqhKeyReq {
    */
   static UintR getDisableFkConstraints(const UintR & requestInfo);
   static void setDisableFkConstraints(UintR & requestInfo, UintR val);
+
+  /**
+   * Get mask of currently undefined bits
+   */
+  static UintR getLongClearBits(const UintR& requestInfo);
 };
 
 /**
@@ -257,6 +262,17 @@ class LqhKeyReq {
 #define RI_DEFERRED_CONSTAINTS (26)
 #define RI_DISABLE_FK        (0)
 
+/* Currently unused */
+#define RI_CLEAR_SHIFT1      (1)
+#define RI_CLEAR_SHIFT2      (2)
+#define RI_CLEAR_SHIFT3      (3)
+#define RI_CLEAR_SHIFT4      (4)
+#define RI_CLEAR_SHIFT5      (5)
+#define RI_CLEAR_SHIFT6      (6)
+#define RI_CLEAR_SHIFT7      (7)
+#define RI_CLEAR_SHIFT8      (8)
+#define RI_CLEAR_SHIFT9      (9)
+
 /**
  * Scan Info
  *
@@ -677,6 +693,24 @@ LqhKeyReq::getDisableFkConstraints(const UintR & requestInfo){
   return (requestInfo >> RI_DISABLE_FK) & 1;
 }
 
+inline
+UintR
+LqhKeyReq::getLongClearBits(const UintR& requestInfo)
+{
+  const Uint32 mask =
+    (1 << RI_CLEAR_SHIFT1) |
+    (1 << RI_CLEAR_SHIFT2) |
+    (1 << RI_CLEAR_SHIFT3) |
+    (1 << RI_CLEAR_SHIFT4) |
+    (1 << RI_CLEAR_SHIFT5) |
+    (1 << RI_CLEAR_SHIFT6) |
+    (1 << RI_CLEAR_SHIFT7) |
+    (1 << RI_CLEAR_SHIFT8) |
+    (1 << RI_CLEAR_SHIFT9);
+
+  return (requestInfo & mask);
+}
+
 inline
 Uint32
 table_version_major_lqhkeyreq(Uint32 x)

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

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+   Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -806,7 +806,7 @@ void Dblqh::execNDB_STTOR(Signal* signal)
   case ZSTART_PHASE1:
     jam();
     preComputedRequestInfoMask = 0;
-    LqhKeyReq::setKeyLen(preComputedRequestInfoMask, RI_KEYLEN_MASK);
+    // Dont setDisableFkconstraints - handled on primary
     LqhKeyReq::setLastReplicaNo(preComputedRequestInfoMask, RI_LAST_REPL_MASK);
     // Dont LqhKeyReq::setApplicationAddressFlag
     LqhKeyReq::setDirtyFlag(preComputedRequestInfoMask, 1);
@@ -4916,6 +4916,24 @@ void Dblqh::execLQHKEYREQ(Signal* signal)
       (Operation_t) op == ZUNLOCK ? ZREAD : // lockType not relevant for unlock req
       (Operation_t) op;
   }
+#ifdef VM_TRACE
+  if (unlikely(isLongReq &&
+               LqhKeyReq::getLongClearBits(Treqinfo) != 0))
+  {
+    jam();
+    /* Bits set which should not be - definite error on same version */
+    const Uint32 ownVersion = getNodeInfo(getOwnNodeId()).m_version;
+    if (senderVersion == ownVersion)
+    {
+      jam();
+      ndbout_c("Received bad long request info %x from same version node %x %x",
+               Treqinfo,
+               senderVersion,
+               ownVersion);
+      ndbrequire(false);
+    }
+  }
+#endif
 
   if (regTcPtr->dirtyOp)
   {
@@ -13349,8 +13367,6 @@ void Dblqh::copyTupkeyConfLab(Signal* signal)
     return;
   }
 
-  LqhKeyReq::setKeyLen(tcConP->reqinfo, len);
-
 /*---------------------------------------------------------------------------*/
 // To avoid using up to many operation records in ACC we will increase the
 // constant to ensure that we never send more than 40 records at a time.
@@ -21843,7 +21859,6 @@ void Dblqh::initReqinfoExecSr(Signal* signal)
 {
   UintR Treqinfo = 0;
   TcConnectionrec * const regTcPtr = tcConnectptr.p;
-  LqhKeyReq::setKeyLen(Treqinfo, regTcPtr->primKeyLen);
 /* ------------------------------------------------------------------------- */
 /* NUMBER OF BACKUPS AND STANDBYS ARE ZERO AND NEED NOT BE SET.              */
 /* REPLICA TYPE IS CLEARED BY SEND_LQHKEYREQ.                                */