diff --git a/libmodprobe/include/modprobe/modprobe.h b/libmodprobe/include/modprobe/modprobe.h index baee4f930..c934860ec 100644 --- a/libmodprobe/include/modprobe/modprobe.h +++ b/libmodprobe/include/modprobe/modprobe.h @@ -24,7 +24,8 @@ class Modprobe { public: - Modprobe(const std::vector&, const std::string load_file = "modules.load"); + Modprobe(const std::vector&, 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 @@ class Modprobe { std::vector* 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 @@ class Modprobe { 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& args); bool ParseAliasCallback(const std::vector& args); diff --git a/libmodprobe/libmodprobe.cpp b/libmodprobe/libmodprobe.cpp index b3ae93785..1a9d3642c 100644 --- a/libmodprobe/libmodprobe.cpp +++ b/libmodprobe/libmodprobe.cpp @@ -313,7 +313,9 @@ void Modprobe::ParseKernelCmdlineOptions(void) { } } -Modprobe::Modprobe(const std::vector& base_paths, const std::string load_file) { +Modprobe::Modprobe(const std::vector& 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 @@ Modprobe::Modprobe(const std::vector& base_paths, const std::string ParseKernelCmdlineOptions(); } -void Modprobe::EnableBlocklist(bool enable) { - blocklist_enabled = enable; -} - std::vector Modprobe::GetDependencies(const std::string& module) { auto it = module_deps_.find(module); if (it == module_deps_.end()) { @@ -427,10 +425,23 @@ bool Modprobe::LoadWithAliases(const std::string& module_name, bool strict, 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::LoadListedModules(bool strict) { 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 d50c10d8f..f960b6139 100644 --- a/libmodprobe/libmodprobe_test.cpp +++ b/libmodprobe/libmodprobe_test.cpp @@ -78,6 +78,18 @@ TEST(libmodprobe, Test) { "/test13.ko", }; + std::vector 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 @@ TEST(libmodprobe, Test) { *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 @@ TEST(libmodprobe, Test) { 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 7529cb920..5e2c5813f 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 7df7b71a9..711586a98 100644 --- a/toolbox/modprobe.cpp +++ b/toolbox/modprobe.cpp @@ -215,10 +215,7 @@ extern "C" int modprobe_main(int argc, char** argv) { 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) {