Merge changes I5989f49c,I394b5aa9
* changes:
Update owners for modprobe.c
libmodprobe: refactor blocklist functionality
diff --git a/libmodprobe/include/modprobe/modprobe.h b/libmodprobe/include/modprobe/modprobe.h
index baee4f9..c934860 100644
--- a/libmodprobe/include/modprobe/modprobe.h
+++ b/libmodprobe/include/modprobe/modprobe.h
@@ -24,7 +24,8 @@
class Modprobe {
public:
- Modprobe(const std::vector<std::string>&, const std::string load_file = "modules.load");
+ Modprobe(const std::vector<std::string>&, const std::string load_file = "modules.load",
+ bool use_blocklist = true);
bool LoadListedModules(bool strict = true);
bool LoadWithAliases(const std::string& module_name, bool strict,
@@ -36,7 +37,6 @@
std::vector<std::string>* post_dependencies);
void ResetModuleCount() { module_count_ = 0; }
int GetModuleCount() { return module_count_; }
- void EnableBlocklist(bool enable);
private:
std::string MakeCanonical(const std::string& module_path);
@@ -48,6 +48,7 @@
void AddOption(const std::string& module_name, const std::string& option_name,
const std::string& value);
std::string GetKernelCmdline();
+ bool IsBlocklisted(const std::string& module_name);
bool ParseDepCallback(const std::string& base_path, const std::vector<std::string>& args);
bool ParseAliasCallback(const std::vector<std::string>& args);
diff --git a/libmodprobe/libmodprobe.cpp b/libmodprobe/libmodprobe.cpp
index b3ae937..1a9d364 100644
--- a/libmodprobe/libmodprobe.cpp
+++ b/libmodprobe/libmodprobe.cpp
@@ -313,7 +313,9 @@
}
}
-Modprobe::Modprobe(const std::vector<std::string>& base_paths, const std::string load_file) {
+Modprobe::Modprobe(const std::vector<std::string>& base_paths, const std::string load_file,
+ bool use_blocklist)
+ : blocklist_enabled(use_blocklist) {
using namespace std::placeholders;
for (const auto& base_path : base_paths) {
@@ -339,10 +341,6 @@
ParseKernelCmdlineOptions();
}
-void Modprobe::EnableBlocklist(bool enable) {
- blocklist_enabled = enable;
-}
-
std::vector<std::string> Modprobe::GetDependencies(const std::string& module) {
auto it = module_deps_.find(module);
if (it == module_deps_.end()) {
@@ -427,10 +425,23 @@
return true;
}
+bool Modprobe::IsBlocklisted(const std::string& module_name) {
+ if (!blocklist_enabled) return false;
+
+ auto canonical_name = MakeCanonical(module_name);
+ auto dependencies = GetDependencies(canonical_name);
+ for (auto dep = dependencies.begin(); dep != dependencies.end(); ++dep) {
+ if (module_blocklist_.count(MakeCanonical(*dep))) return true;
+ }
+
+ return module_blocklist_.count(canonical_name) > 0;
+}
+
bool Modprobe::LoadListedModules(bool strict) {
auto ret = true;
for (const auto& module : module_load_) {
if (!LoadWithAliases(module, true)) {
+ if (IsBlocklisted(module)) continue;
ret = false;
if (strict) break;
}
@@ -440,16 +451,10 @@
bool Modprobe::Remove(const std::string& module_name) {
auto dependencies = GetDependencies(MakeCanonical(module_name));
- if (dependencies.empty()) {
- LOG(ERROR) << "Empty dependencies for module " << module_name;
- return false;
- }
- if (!Rmmod(dependencies[0])) {
- return false;
- }
- for (auto dep = dependencies.begin() + 1; dep != dependencies.end(); ++dep) {
+ for (auto dep = dependencies.begin(); dep != dependencies.end(); ++dep) {
Rmmod(*dep);
}
+ Rmmod(module_name);
return true;
}
diff --git a/libmodprobe/libmodprobe_test.cpp b/libmodprobe/libmodprobe_test.cpp
index d50c10d..f960b61 100644
--- a/libmodprobe/libmodprobe_test.cpp
+++ b/libmodprobe/libmodprobe_test.cpp
@@ -78,6 +78,18 @@
"/test13.ko",
};
+ std::vector<std::string> expected_modules_blocklist_enabled = {
+ "/test1.ko option1=50 option2=60",
+ "/test6.ko",
+ "/test2.ko",
+ "/test5.ko option1=",
+ "/test8.ko",
+ "/test7.ko param1=4",
+ "/test12.ko",
+ "/test11.ko",
+ "/test13.ko",
+ };
+
const std::string modules_dep =
"test1.ko:\n"
"test2.ko:\n"
@@ -146,7 +158,7 @@
*i = dir.path + *i;
}
- Modprobe m({dir.path});
+ Modprobe m({dir.path}, "modules.load", false);
EXPECT_TRUE(m.LoadListedModules());
GTEST_LOG_(INFO) << "Expected modules loaded (in order):";
@@ -176,8 +188,22 @@
EXPECT_TRUE(modules_loaded == expected_after_remove);
- m.EnableBlocklist(true);
+ m = Modprobe({dir.path});
EXPECT_FALSE(m.LoadWithAliases("test4", true));
+ while (modules_loaded.size() > 0) EXPECT_TRUE(m.Remove(modules_loaded.front()));
+ EXPECT_TRUE(m.LoadListedModules());
+
+ GTEST_LOG_(INFO) << "Expected modules loaded after enabling blocklist (in order):";
+ for (auto i = expected_modules_blocklist_enabled.begin();
+ i != expected_modules_blocklist_enabled.end(); ++i) {
+ *i = dir.path + *i;
+ GTEST_LOG_(INFO) << "\"" << *i << "\"";
+ }
+ GTEST_LOG_(INFO) << "Actual modules loaded with blocklist enabled (in order):";
+ for (auto i = modules_loaded.begin(); i != modules_loaded.end(); ++i) {
+ GTEST_LOG_(INFO) << "\"" << *i << "\"";
+ }
+ EXPECT_TRUE(modules_loaded == expected_modules_blocklist_enabled);
}
TEST(libmodprobe, ModuleDepLineWithoutColonIsSkipped) {
diff --git a/toolbox/OWNERS b/toolbox/OWNERS
index 7529cb9..5e2c581 100644
--- a/toolbox/OWNERS
+++ b/toolbox/OWNERS
@@ -1 +1,2 @@
include platform/system/core:/janitors/OWNERS
+per-file modprobe.c=willmcvicker@google.com,dvander@google.com
diff --git a/toolbox/modprobe.cpp b/toolbox/modprobe.cpp
index 7df7b71..711586a 100644
--- a/toolbox/modprobe.cpp
+++ b/toolbox/modprobe.cpp
@@ -215,10 +215,7 @@
return EXIT_FAILURE;
}
- Modprobe m(mod_dirs);
- if (blocklist) {
- m.EnableBlocklist(true);
- }
+ Modprobe m(mod_dirs, "modules.load", blocklist);
for (const auto& module : modules) {
switch (mode) {