[SERVER-39490] opWriteConcernCounters can cause undefined behavior due to overflow Created: 11/Feb/19 Updated: 29/Oct/23 Resolved: 06/Mar/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | 3.6.11, 4.0.6, 4.1.7 |
| Fix Version/s: | 3.6.12, 4.0.7, 4.1.9 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Tess Avitabile (Inactive) | Assignee: | Tess Avitabile (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Backwards Compatibility: | Fully Compatible | ||||
| Operating System: | ALL | ||||
| Backport Requested: |
v4.0, v3.6
|
||||
| Sprint: | Repl 2019-03-11 | ||||
| Participants: | |||||
| Description |
|
The counters in ServerWriteConcernMetrics can overflow, since they are signed integers, and they are not protected by std::atomic. We should make these counters unsigned (preferably std::uint64_t). In order to append them to a BSONObj here, we can cast them to long long. |
| Comments |
| Comment by Githook User [ 11/Mar/19 ] |
|
Author: {'name': 'Tess Avitabile', 'email': 'tess.avitabile@mongodb.com', 'username': 'tessavitabile'}Message: (cherry picked from commit 884ac2862eda54e15d99e4db253b822cb4a90f1d) |
| Comment by Githook User [ 11/Mar/19 ] |
|
Author: {'name': 'Tess Avitabile', 'username': 'tessavitabile', 'email': 'tess.avitabile@mongodb.com'}Message: (cherry picked from commit 884ac2862eda54e15d99e4db253b822cb4a90f1d) |
| Comment by Githook User [ 06/Mar/19 ] |
|
Author: {'name': 'Tess Avitabile', 'email': 'tess.avitabile@mongodb.com', 'username': 'tessavitabile'}Message: |
| Comment by Tess Avitabile (Inactive) [ 13/Feb/19 ] |
|
Updated the ticket description based on discussion with acm. Although we are probably unlikely to overflow a 64-bit integer in practice, the compiler is free to assume that signed overflow simply does not happen, and reason from that assumption, so it is better to avoid the risk of undefined behavior. The ServerReadConcernMetrics are not affected because they are unsigned and they are protected by std::atomic (either of these would be sufficient). |
| Comment by Tess Avitabile (Inactive) [ 12/Feb/19 ] |
|
geert.bosch suggested that we do not have to fix this issue, since we only increment the counters, and the counters are 64 bits. |