Commit affe1977 authored by Mike Wittman's avatar Mike Wittman Committed by Commit Bot

Add ModuleCache API contract checks

Adds checks to ensure early detection of API contract violations that
would lead to use-after-frees.

Bug: 1127466
Change-Id: Ic73b30f115f5206572a427ac44674ff6a9b9f249
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425523
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: default avatarEtienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809987}
parent d8d9f696
...@@ -39,8 +39,11 @@ const ModuleCache::Module* ModuleCache::GetModuleForAddress(uintptr_t address) { ...@@ -39,8 +39,11 @@ const ModuleCache::Module* ModuleCache::GetModuleForAddress(uintptr_t address) {
std::unique_ptr<const Module> new_module = CreateModuleForAddress(address); std::unique_ptr<const Module> new_module = CreateModuleForAddress(address);
if (!new_module) if (!new_module)
return nullptr; return nullptr;
const auto loc = native_modules_.insert(std::move(new_module)); const auto result = native_modules_.insert(std::move(new_module));
return loc.first->get(); // Ensure that the new module was inserted an isn't equivalent to an existing
// module.
DCHECK(result.second);
return result.first->get();
} }
std::vector<const ModuleCache::Module*> ModuleCache::GetModules() const { std::vector<const ModuleCache::Module*> ModuleCache::GetModules() const {
...@@ -87,12 +90,27 @@ void ModuleCache::UpdateNonNativeModules( ...@@ -87,12 +90,27 @@ void ModuleCache::UpdateNonNativeModules(
// Insert the modules to be added. This operation is O((m + a) + a*log(a)) // Insert the modules to be added. This operation is O((m + a) + a*log(a))
// where m is the number of current modules and a is the number of modules to // where m is the number of current modules and a is the number of modules to
// be added. // be added.
const size_t prior_non_native_modules_size = non_native_modules_.size();
non_native_modules_.insert(std::make_move_iterator(new_modules.begin()), non_native_modules_.insert(std::make_move_iterator(new_modules.begin()),
std::make_move_iterator(new_modules.end())); std::make_move_iterator(new_modules.end()));
// Every module in |new_modules| should have been moved into
// |non_native_modules_|. This guards against use-after-frees if |new_modules|
// were to contain any modules equivalent to what's already in
// |non_native_modules_|, in which case the module would remain in
// |new_modules| and be deleted on return from the function. While this
// scenario would be a violation of the API contract, it would present a
// difficult-to-track-down crash scenario.
CHECK_EQ(prior_non_native_modules_size + new_modules.size(),
non_native_modules_.size());
} }
void ModuleCache::AddCustomNativeModule(std::unique_ptr<const Module> module) { void ModuleCache::AddCustomNativeModule(std::unique_ptr<const Module> module) {
native_modules_.insert(std::move(module)); const bool was_inserted = native_modules_.insert(std::move(module)).second;
// |module| should have been inserted into |native_modules_|, indicating that
// there was no equivalent module already present. While this scenario would
// be a violation of the API contract, it would present a
// difficult-to-track-down crash scenario.
CHECK(was_inserted);
} }
const ModuleCache::Module* ModuleCache::GetExistingModuleForAddress( const ModuleCache::Module* ModuleCache::GetExistingModuleForAddress(
......
...@@ -88,7 +88,10 @@ class BASE_EXPORT ModuleCache { ...@@ -88,7 +88,10 @@ class BASE_EXPORT ModuleCache {
// specifically they no longer participate in the GetModuleForAddress() // specifically they no longer participate in the GetModuleForAddress()
// lookup. They continue to exist for the lifetime of the ModuleCache, // lookup. They continue to exist for the lifetime of the ModuleCache,
// however, so that existing references to them remain valid. Modules in // however, so that existing references to them remain valid. Modules in
// |new_modules| are added to the set of active non-native modules. // |new_modules| are added to the set of active non-native modules. Modules in
// |new_modules| may not overlap with any non-native Modules already present
// in ModuleCache, unless those modules are provided in |defunct_modules| in
// the same call.
void UpdateNonNativeModules( void UpdateNonNativeModules(
const std::vector<const Module*>& defunct_modules, const std::vector<const Module*>& defunct_modules,
std::vector<std::unique_ptr<const Module>> new_modules); std::vector<std::unique_ptr<const Module>> new_modules);
...@@ -96,6 +99,8 @@ class BASE_EXPORT ModuleCache { ...@@ -96,6 +99,8 @@ class BASE_EXPORT ModuleCache {
// Adds a custom native module to the cache. This is intended to support // Adds a custom native module to the cache. This is intended to support
// native modules that require custom handling. In general, native modules // native modules that require custom handling. In general, native modules
// will be found and added automatically when invoking GetModuleForAddress(). // will be found and added automatically when invoking GetModuleForAddress().
// |module| may not overlap with any native Modules already present in
// ModuleCache.
void AddCustomNativeModule(std::unique_ptr<const Module> module); void AddCustomNativeModule(std::unique_ptr<const Module> module);
// Gets the module containing |address| if one already exists, or nullptr // Gets the module containing |address| if one already exists, or nullptr
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment