Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-22569

Initialization of eooElement static local variable isn't thread safe with MSVC 2013

    • Fully Compatible
    • Windows
    • Query 10 (02/22/16)
    • 0

      As part of the changes from SERVER-14973, a const reference to a BSONElement of type EOO is returned by the findParentEqualityElement() function when there aren't any equality matches along any component of the path.

      const BSONElement& findParentEqualityElement(const EqualityMatches& equalities,
                                                   const FieldRef& path,
                                                   int* parentPathParts) {
          // We may have an equality match to an object at a higher point in the pattern path, check
          // all path prefixes for equality matches
          // ex: path: 'a.b', query : { 'a' : { b : <value> } }
          // ex: path: 'a.b.c', query : { 'a.b' : { c : <value> } }
          for (int i = static_cast<int>(path.numParts()); i >= 0; --i) {
              // "" element is *not* a parent of anyone but itself
              if (i == 0 && path.numParts() != 0)
                  continue;
      
              StringData subPathStr = path.dottedSubstring(0, i);
              EqualityMatches::const_iterator seenIt = equalities.find(subPathStr);
              if (seenIt == equalities.end())
                  continue;
      
              *parentPathParts = i;
              return seenIt->second->getData();
          }
      
          *parentPathParts = -1;
          static const BSONElement eooElement;
          return eooElement;
      }
      

      Due to the lack of thread-safe function local static initialization support in MSVC 2013, it's possible for the findParentEqualityElement() function to be called by multiple threads concurrently and for one of the threads to return a reference to uninitialized memory.

      Analysis from the core dump attached to this ticket

      Below are the values of the local variables in the checkEqualityConflicts() function. It's peculiar that parentEl isn't present in this list; however, the fact that parentPathPart == -1 indicates that the findParentEqualityElement() function returned eooElement.

      Name Value Type
      equalities {size=2234102074524566271} const std::map<mongo::StringData,mongo::EqualityMatchExpression const *,std::less<mongo::StringData>,std::allocator<std::pair<mongo::StringData const,mongo::EqualityMatchExpression const *> > > &
      path {_size=285537879520 _fixed=0x000000427d98c1f8 {{_data=0x0000000000000000 <NULL> _size=0}, {_data=0x0000000000000000 <NULL> ...}, ...} ...} const mongo::FieldRef &
      suffixStr {_data=0x000007f7b78d8e22 "HƒÄ(ÃÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌff\xf\x1f„" _size=0} mongo::StringData
      prefixStr {_data=0x000000427d98c888 "È˜}B" _size=285575004344} mongo::StringData
      pathStr {_data=0x0000004200000000 "" _size=0} mongo::StringData
      errMsg "" std::basic_string<char,std::char_traits<char>,std::allocator<char> >
      parentPathPart -1 int

      Below is the assembly code corresponding to these lines of findParentEqualityElement(). The dword ptr [eooElement+10h (07F7B7D3C940h)] address is set to 1 when eooElement is being initialized and not after it has been initialized. It's then possible that another thread performs test al,1 and returns [eooElement (07F7B7D3C930h)] prior to the BSONElement value being fully initialized.

          *parentPathParts = -1;
      000007F7B73FA3B2  mov         dword ptr [r15],0FFFFFFFFh
          static const BSONElement eooElement;
      000007F7B73FA3B9  mov         eax,dword ptr [eooElement+10h (07F7B7D3C940h)]
      000007F7B73FA3BF  test        al,1
      000007F7B73FA3C1  jne         mongo::pathsupport::findParentEqualityElement+36Bh (07F7B73FA3EBh)
      000007F7B73FA3C3  or          eax,1
      000007F7B73FA3C6  mov         dword ptr [eooElement+10h (07F7B7D3C940h)],eax
      000007F7B73FA3CC  lea         rax,[`mongo::BSONElement::BSONElement'::`2'::kEooElement (07F7B7A0DF68h)]
      000007F7B73FA3D3  mov         qword ptr [eooElement (07F7B7D3C930h)],rax
      000007F7B73FA3DA  mov         dword ptr [eooElement+8h (07F7B7D3C938h)],r13d
      000007F7B73FA3E1  mov         dword ptr [eooElement+0Ch (07F7B7D3C93Ch)],1
          return eooElement;
      000007F7B73FA3EB  lea         rax,[eooElement (07F7B7D3C930h)]
      }
      000007F7B73FA3F2  mov         rbx,qword ptr [equalities]
      000007F7B73FA3FA  add         rsp,80h
      000007F7B73FA401  pop         r15
      000007F7B73FA403  pop         r14
      000007F7B73FA405  pop         r13
      000007F7B73FA407  pop         r12
      000007F7B73FA409  pop         rdi
      000007F7B73FA40A  pop         rsi
      000007F7B73FA40B  pop         rbp
      000007F7B73FA40C  ret
      

            Assignee:
            max.hirschhorn@mongodb.com Max Hirschhorn
            Reporter:
            max.hirschhorn@mongodb.com Max Hirschhorn
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: