[SERVER-81248] parseMaxTimeMS may narrow values of maxTimeMSOpOnly Created: 20/Sep/23  Updated: 03/Oct/23

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Minor - P4
Reporter: James Harrison Assignee: Backlog - Query Optimization
Resolution: Unresolved Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Query Optimization
Operating System: ALL
Participants:

 Description   

When used to parse a field with the name maxTimeMSOpOnly, parseMaxTimeMS increases the max permitted value beyond INT_MAX.

However, the returned value is of type int - the result will be narrowed long long -> int.

StatusWith<int> parseMaxTimeMS(BSONElement maxTimeMSElt) {
...
    const long long maxVal = maxTimeMSElt.fieldNameStringData() == kMaxTimeMSOpOnlyField
        ? (long long)(INT_MAX) + kMaxTimeMSOpOnlyMaxPadding
        : INT_MAX;
...
    if (maxTimeMSLongLong < 0 || maxTimeMSLongLong > maxVal)
        return Status(ErrorCodes::BadValue, ...);
..
    return StatusWith<int>(static_cast<int>(maxTimeMSLongLong));
}

The justification for this leeway:

src/mongo/db/query/max_time_ms_parser.h

43
// A constant by which 'maxTimeMSOpOnly' values are allowed to exceed the max allowed value for
44
// 'maxTimeMS'.  This is because mongod and mongos server processes add a small amount to the
45
// 'maxTimeMS' value they are given before passing it on as 'maxTimeMSOpOnly', to allow for
46
// clock precision.
47
static constexpr auto kMaxTimeMSOpOnlyMaxPadding = 100LL;

Given this, if maxTimeMS is provided INT_MAX, any maxTimeMSOpOnly derived from increasing this value slightly will be parsed as a very negative int value.

The only use of this method on a field named maxTimeMSOpOnly identified so far is:

src/mongo/db/service_entry_point_common.cpp

    void ExecCommandDatabase::_initiateCommand() {
...
        const auto maxTimeMS =
            Milliseconds{uassertStatusOK(parseMaxTimeMS(cmdOptionMaxTimeMSField))};
        const auto maxTimeMSOpOnly =
            Milliseconds{uassertStatusOK(parseMaxTimeMS(maxTimeMSOpOnlyField))};
 
        if ((maxTimeMS > Milliseconds::zero() || maxTimeMSOpOnly > Milliseconds::zero()) &&
            command->getLogicalOp() != LogicalOp::opGetMore) {
           ...
 
            if (!ignoreMaxTimeMSOpOnly && maxTimeMSOpOnly > Milliseconds::zero() &&
                (maxTimeMS == Milliseconds::zero() || maxTimeMSOpOnly < maxTimeMS)) {
                opCtx->storeMaxTimeMS(maxTimeMS);
                opCtx->setDeadlineByDate(startedCommandExecAt + maxTimeMSOpOnly,
                                         ErrorCodes::MaxTimeMSExpired);
            } else if (maxTimeMS > Milliseconds::zero()) {
                opCtx->setDeadlineByDate(startedCommandExecAt + maxTimeMS,
                                         ErrorCodes::MaxTimeMSExpired);
            }
        }

This compares > 0, so may treat this situation as if maxTimeMSOpOnly were 0 or unset.

Given INT_MAX milliseconds is around 25 days, it's possible this has no real-world impact.


Generated at Thu Feb 08 06:45:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.