[SERVER-22569] Initialization of eooElement static local variable isn't thread safe with MSVC 2013 Created: 10/Feb/16  Updated: 17/Nov/16  Resolved: 12/Feb/16

Status: Closed
Project: Core Server
Component/s: Querying, Sharding
Affects Version/s: None
Fix Version/s: 3.0.10, 3.2.4, 3.3.2

Type: Bug Priority: Major - P3
Reporter: Max Hirschhorn Assignee: Max Hirschhorn
Resolution: Done Votes: 0
Labels: code-only
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File mongos.2016-01-20T15-24-02.mdmp    
Issue Links:
Depends
Backwards Compatibility: Fully Compatible
Operating System: Windows
Backport Completed:
Sprint: Query 10 (02/22/16)
Participants:
Linked BF Score: 0

 Description   

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



 Comments   
Comment by Githook User [ 20/Feb/16 ]

Author:

{u'username': u'visemet', u'name': u'Max Hirschhorn', u'email': u'max.hirschhorn@mongodb.com'}

Message: SERVER-22569 Change findParentEqualityElement() to return by value.

MSVC 2013 doesn't support thread-safe initialization of function-local
static-duration objects, so it's possible to return a reference to
'eooElement' prior to the value being fully initialized.

(cherry picked from commit 4f1cc51f3e21e4ff76c68e86ecae4e5d138de0aa)
Branch: v3.0
https://github.com/mongodb/mongo/commit/51a2c9eabc44ff9669d11802b9453c1a40e9a32c

Comment by Githook User [ 20/Feb/16 ]

Author:

{u'username': u'visemet', u'name': u'Max Hirschhorn', u'email': u'max.hirschhorn@mongodb.com'}

Message: SERVER-22569 Change findParentEqualityElement() to return by value.

MSVC 2013 doesn't support thread-safe initialization of function-local
static-duration objects, so it's possible to return a reference to
'eooElement' prior to the value being fully initialized.

(cherry picked from commit 4f1cc51f3e21e4ff76c68e86ecae4e5d138de0aa)
Branch: v3.2
https://github.com/mongodb/mongo/commit/f78f332e0748ac79bb40b490eb6e5462c49da9d0

Comment by Githook User [ 12/Feb/16 ]

Author:

{u'username': u'visemet', u'name': u'Max Hirschhorn', u'email': u'max.hirschhorn@mongodb.com'}

Message: SERVER-22569 Change findParentEqualityElement() to return by value.

MSVC 2013 doesn't support thread-safe initialization of function-local
static-duration objects, so it's possible to return a reference to
'eooElement' prior to the value being fully initialized.
Branch: master
https://github.com/mongodb/mongo/commit/4f1cc51f3e21e4ff76c68e86ecae4e5d138de0aa

Generated at Thu Feb 08 04:00:46 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.