Uploaded image for project: 'WiredTiger'
  1. WiredTiger
  2. WT-6120

Remove use-after-free in __verify_history_store_id

    XMLWordPrintable

    Details

    • Type: Build Failure
    • Status: Closed
    • Priority: Major - P3
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: WT10.0.0, 4.4.0-rc4, 4.7.0
    • Component/s: None
    • Labels:
      None
    • Story Points:
      5
    • Sprint:
      Storage - Ra 2020-05-18

      Description

      This signature has been appearing often in MongoDB tests that are compiled with address sanitizer.

      =================================================================
      ==54784==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000228832 at pc 0x5557701d4b1a bp 0x7efbb1b07510 sp 0x7efbb1b07508
      READ of size 1 at 0x602000228832 thread T335 (conn263)
      #0 0x5557701d4b19 in __verify_history_store_id (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2dabb19)
      #1 0x5557701d2857 in __wt_history_store_verify_one (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2da9857)
      #2 0x5557704d1262 in __wt_verify (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x30a8262)
      #3 0x555770290b6f in __wt_exclusive_handle_operation (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2e67b6f)
      #4 0x555770290fcf in __wt_schema_worker (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2e67fcf)
      #5 0x55577029126a in __wt_schema_worker (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2e6826a)
      #6 0x5557702c1390 in __session_verify (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2e98390)
      #7 0x5557700f759d in mongo::WiredTigerUtil::verifyTable(mongo::OperationContext*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >*) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2cce59d)
      #8 0x55577006d998 in mongo::WiredTigerRecordStore::validate(mongo::OperationContext*, mongo::ValidateResults*, mongo::BSONObjBuilder*) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x2c44998)
      #9 0x5557717f7dac in mongo::CollectionValidation::validate(mongo::OperationContext*, mongo::NamespaceString const&, mongo::CollectionValidation::ValidateOptions, bool, mongo::ValidateResults*, mongo::BSONObjBuilder*, bool) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x43cedac)
      #10 0x5557717b7dcc in mongo::ValidateCmd::run(mongo::OperationContext*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, mongo::BSONObj const&, mongo::BSONObjBuilder&) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x438edcc)
      #11 0x55576f4b8f35 in mongo::BasicCommand::runWithReplyBuilder(mongo::OperationContext*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, mongo::BSONObj const&, mongo::rpc::ReplyBuilderInterface*) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x208ff35)
      #12 0x555773019f7d in mongo::BasicCommandWithReplyBuilderInterface::Invocation::run(mongo::OperationContext*, mongo::rpc::ReplyBuilderInterface*) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x5bf0f7d)
      #13 0x555772ff61da in mongo::CommandHelpers::runCommandInvocation(mongo::OperationContext*, mongo::OpMsgRequest const&, mongo::CommandInvocation*, mongo::rpc::ReplyBuilderInterface*) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x5bcd1da)
      #14 0x5557708aab20 in mongo::(anonymous namespace)::execCommandDatabase(mongo::OperationContext*, mongo::Command*, mongo::OpMsgRequest const&, mongo::rpc::ReplyBuilderInterface*, mongo::ServiceEntryPointCommon::Hooks const&) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x3481b20)
      #15 0x5557708903cf in mongo::ServiceEntryPointCommon::handleRequest(mongo::OperationContext*, mongo::Message const&, mongo::ServiceEntryPointCommon::Hooks const&) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x34673cf)
      #16 0x555770858f3a in mongo::ServiceEntryPointMongod::handleRequest(mongo::OperationContext*, mongo::Message const&) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x342ff3a)
      #17 0x555770883456 in mongo::ServiceStateMachine::_processMessage(mongo::ServiceStateMachine::ThreadGuard) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x345a456)
      #18 0x5557708766e5 in mongo::ServiceStateMachine::_runNextInGuard(mongo::ServiceStateMachine::ThreadGuard) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x344d6e5)
      #19 0x5557708813eb in std::_Function_handler<void (), mongo::ServiceStateMachine::_scheduleNextWithGuard(mongo::ServiceStateMachine::ThreadGuard, mongo::transport::ServiceExecutor::ScheduleFlags, mongo::transport::ServiceExecutorTaskName, mongo::ServiceStateMachine::Ownership)::$_6>::_M_invoke(std::_Any_data const&) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x34583eb)
      #20 0x555775e7d537 in mongo::transport::ServiceExecutorSynchronous::schedule(std::function<void ()>, mongo::transport::ServiceExecutor::ScheduleFlags, mongo::transport::ServiceExecutorTaskName) (/data/mci/01f09dc8d3c96dbdaf11615b4f6fb2a2/src/dist-test/bin/mongod+0x8a54537)
      

      It looks to me that in __verify_history_store_id, prev_hs_key points to some memory in the cursor buffer which we proceed to invalidate with the next call and then attempt to read from. Even though prev_hs_key is a scratch buffer, when we use it in the get_key call, it simply points the data buffer at something in the cursor's memory.

      If I do this, ASan doesn't complain (since we're memcpying the contents into prev_hs_key's own buffer).

      modified   src/third_party/wiredtiger/src/history/hs.c
      @@ -1314,6 +1314,7 @@ __verify_history_store_id(WT_SESSION_IMPL *session, WT_CURSOR_BTREE *cbt, uint32
               tmp = hs_key;
               hs_key = prev_hs_key;
               prev_hs_key = tmp;
      +        WT_ERR(__wt_buf_set(session, prev_hs_key, prev_hs_key->data, prev_hs_key->size));
           }
           WT_ERR_NOTFOUND_OK(ret, true);
       err:
      

      I believe this got introduced in this refactor: https://github.com/wiredtiger/wiredtiger/commit/1df68a4888dfd10b1966c6b88800e655efc84fa3

      While we're here, we should also reevaluate our use of scratch buffers in hs.c. There is no advantage in using them unless we're persisting them over the next iteration since WT_ITEMs are never pointing at memory they own. So for example, I think hs_key in this function doesn't need to be a scratch buffer.

        Attachments

          Activity

            People

            Assignee:
            alex.cameron Alex Cameron (Inactive)
            Reporter:
            alex.cameron Alex Cameron (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: