libmodprobe: add missing locks in LoadWithAliases
`Modprobe::LoadWithAliases` searches `module_loaded_` for a new element
without holding the lock of `module_loaded_lock_`. However,
`InsmodWithDeps` will insert an element later in this function, and
`LoadWithAliases` can be called by `LoadModulesParallel` in parallel.
Therefore `LoadWithAliases` touches `module_loaded_` while it's being
modified by other threads. That means it accesses corrputed memory.
This change inserts a necessary lock to fix this bug of
`LoadModulesParallel` and `LoadWithAliases`. This change does not
include changes to add thread safety annotations to prevent future same
problem, but following other changes will do that.
Bug: 400592897
Test: cuttlefish boots
Change-Id: Ib52a3b147cbe4122ae42c9fd377e9ea728a4aa41
diff --git a/libmodprobe/libmodprobe.cpp b/libmodprobe/libmodprobe.cpp
index bdd114c..f74e15f 100644
--- a/libmodprobe/libmodprobe.cpp
+++ b/libmodprobe/libmodprobe.cpp
@@ -458,21 +458,24 @@
bool Modprobe::LoadWithAliases(const std::string& module_name, bool strict,
const std::string& parameters) {
- auto canonical_name = MakeCanonical(module_name);
- if (module_loaded_.count(canonical_name)) {
- return true;
- }
-
- std::set<std::string> modules_to_load = {canonical_name};
+ std::set<std::string> modules_to_load;
bool module_loaded = false;
+ {
+ std::lock_guard guard(module_loaded_lock_);
- // use aliases to expand list of modules to load (multiple modules
- // may alias themselves to the requested name)
- for (const auto& [alias, aliased_module] : module_aliases_) {
- if (fnmatch(alias.c_str(), module_name.c_str(), 0) != 0) continue;
- LOG(VERBOSE) << "Found alias for '" << module_name << "': '" << aliased_module;
- if (module_loaded_.count(MakeCanonical(aliased_module))) continue;
- modules_to_load.emplace(aliased_module);
+ auto canonical_name = MakeCanonical(module_name);
+ if (module_loaded_.count(canonical_name)) {
+ return true;
+ }
+ modules_to_load.insert(std::move(canonical_name));
+ // use aliases to expand list of modules to load (multiple modules
+ // may alias themselves to the requested name)
+ for (const auto& [alias, aliased_module] : module_aliases_) {
+ if (fnmatch(alias.c_str(), module_name.c_str(), 0) != 0) continue;
+ LOG(VERBOSE) << "Found alias for '" << module_name << "': '" << aliased_module;
+ if (module_loaded_.count(MakeCanonical(aliased_module))) continue;
+ modules_to_load.emplace(aliased_module);
+ }
}
// attempt to load all modules aliased to this name
