[SERVER-21166] Leak in getExecutorDistinct Created: 27/Oct/15 Updated: 25/Apr/16 Resolved: 29/Oct/15 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Querying |
| Affects Version/s: | 3.2.0-rc1 |
| Fix Version/s: | 3.2.0-rc2 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Robert Guo (Inactive) | Assignee: | David Storch |
| Resolution: | Done | Votes: | 0 |
| Labels: | fuzzer-blocker | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Backwards Compatibility: | Minor Change | ||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||
| Sprint: | TIG B (10/30/15) | ||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||
| Description |
|
mongod githash: 0613deff6f
|
| Comments |
| Comment by Githook User [ 30/Oct/15 ] |
|
Author: {u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}Message: |
| Comment by Githook User [ 29/Oct/15 ] |
|
Author: {u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}Message: |
| Comment by Githook User [ 29/Oct/15 ] |
|
Author: {u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}Message: |
| Comment by David Storch [ 29/Oct/15 ] |
|
The root cause of the leak is an unexpected exception being thrown by a verify() tripped during query planning. We were leaking a raw pointer, so I've changed the raw pointer to a unique_ptr in order to protect against any early return. I've also fixed the verify() failure, which is the underlying problem. Planning code is converting the value inside the index key pattern to a 32-bit signed int in order to determine whether the index is ascending or descending. If the value overflows these 32 bits, then the planner can misinterpret an ascending index as descending (or vice-versa). In order to protect against this scenario, I've added validation which makes sure that the numeric value inside each key pattern element fits inside a 32-bit signed int. |
| Comment by David Storch [ 27/Oct/15 ] |
|
Looks like we're leaking the DistinctNode: We should be managing the DistinctNode with a unique_ptr. However, a repro would be useful, since the call to analyzeDataAccess() is supposed to take ownership of the DistinctNode, and I'm not sure how anything before then could throw. |