From badb7de1a23563dbc7b72f1c1d241c9085d5e4ed Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Tue, 10 May 2022 03:16:51 +0900 Subject: [PATCH 1/3] APEX configs support 'on' as well APEX configs have supported only 'service' definitions. For those services relying on 'on' trigger actions, we had to have separate config files installed in read-only partitions (e.g. /system/etc/init). This was suboptimal because even though APEXes are updatable, read-only partitions are not. Now, 'on' is supported in APEX configs. Putting 'on' trigger actions near to service definitions makes APEX more self-contained. 'on' trigger actions loaded from APEX configs are not sticky. So, events happens before loading APEX configs can't trigger actions. For example, 'post-fs-data' is where APEX configs are loaded for now, so 'on post-fs-data' in APEX configs can't be triggerd. Bug: 202731768 Test: atest CtsInitTestCases Change-Id: I5a01d9c7c57b07955b829d6cc157e7f0c91166f9 --- init/action.h | 3 ++ init/action_manager.h | 4 ++ init/builtins.cpp | 3 +- init/init.cpp | 8 +-- init/init.h | 2 +- init/init_test.cpp | 118 +++++++++++++++++++++++++++++++++++++----- init/reboot.cpp | 11 +++- 7 files changed, 129 insertions(+), 20 deletions(-) diff --git a/init/action.h b/init/action.h index 1534bf987..eddc384cd 100644 --- a/init/action.h +++ b/init/action.h @@ -22,6 +22,8 @@ #include #include +#include + #include "builtins.h" #include "keyword_map.h" #include "result.h" @@ -79,6 +81,7 @@ class Action { static void set_function_map(const BuiltinFunctionMap* function_map) { function_map_ = function_map; } + bool IsFromApex() const { return base::StartsWith(filename_, "/apex/"); } private: void ExecuteCommand(const Command& command) const; diff --git a/init/action_manager.h b/init/action_manager.h index b6f93d9b5..2746a7c4a 100644 --- a/init/action_manager.h +++ b/init/action_manager.h @@ -37,6 +37,10 @@ class ActionManager { size_t CheckAllCommands(); void AddAction(std::unique_ptr action); + template + void RemoveActionIf(UnaryPredicate predicate) { + actions_.erase(std::remove_if(actions_.begin(), actions_.end(), predicate), actions_.end()); + } void QueueEventTrigger(const std::string& trigger); void QueuePropertyChange(const std::string& name, const std::string& value); void QueueAllPropertyActions(); diff --git a/init/builtins.cpp b/init/builtins.cpp index 01db4f5da..9e1d93c6b 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -1288,7 +1288,8 @@ static Result parse_apex_configs() { return Error() << "glob pattern '" << glob_pattern << "' failed"; } std::vector configs; - Parser parser = CreateServiceOnlyParser(ServiceList::GetInstance(), true); + Parser parser = + CreateApexConfigParser(ActionManager::GetInstance(), ServiceList::GetInstance()); for (size_t i = 0; i < glob_result.gl_pathc; i++) { std::string path = glob_result.gl_pathv[i]; // Filter-out /apex/@ paths. The paths are bind-mounted to diff --git a/init/init.cpp b/init/init.cpp index f8330bc5b..fd8ee0fe8 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -293,13 +293,15 @@ Parser CreateParser(ActionManager& action_manager, ServiceList& service_list) { return parser; } -// parser that only accepts new services -Parser CreateServiceOnlyParser(ServiceList& service_list, bool from_apex) { +// Returns a Parser that accepts scripts from APEX modules. It supports `service` and `on`. +Parser CreateApexConfigParser(ActionManager& action_manager, ServiceList& service_list) { Parser parser; parser.AddSectionParser( "service", std::make_unique(&service_list, GetSubcontext(), std::nullopt, - from_apex)); + /*from_apex=*/true)); + parser.AddSectionParser("on", std::make_unique(&action_manager, GetSubcontext())); + return parser; } diff --git a/init/init.h b/init/init.h index 4f686cb5b..522053549 100644 --- a/init/init.h +++ b/init/init.h @@ -29,7 +29,7 @@ namespace android { namespace init { Parser CreateParser(ActionManager& action_manager, ServiceList& service_list); -Parser CreateServiceOnlyParser(ServiceList& service_list, bool from_apex); +Parser CreateApexConfigParser(ActionManager& action_manager, ServiceList& service_list); bool start_waiting_for_property(const char *name, const char *value); diff --git a/init/init_test.cpp b/init/init_test.cpp index 8c19d5f5a..0dc6ff640 100644 --- a/init/init_test.cpp +++ b/init/init_test.cpp @@ -42,34 +42,34 @@ namespace init { using ActionManagerCommand = std::function; void TestInit(const std::string& init_script_file, const BuiltinFunctionMap& test_function_map, - const std::vector& commands, ServiceList* service_list) { - ActionManager am; - + const std::vector& commands, ActionManager* action_manager, + ServiceList* service_list) { Action::set_function_map(&test_function_map); Parser parser; parser.AddSectionParser("service", std::make_unique(service_list, nullptr, std::nullopt)); - parser.AddSectionParser("on", std::make_unique(&am, nullptr)); + parser.AddSectionParser("on", std::make_unique(action_manager, nullptr)); parser.AddSectionParser("import", std::make_unique(&parser)); ASSERT_TRUE(parser.ParseConfig(init_script_file)); for (const auto& command : commands) { - command(am); + command(*action_manager); } - while (am.HasMoreCommands()) { - am.ExecuteOneCommand(); + while (action_manager->HasMoreCommands()) { + action_manager->ExecuteOneCommand(); } } void TestInitText(const std::string& init_script, const BuiltinFunctionMap& test_function_map, - const std::vector& commands, ServiceList* service_list) { + const std::vector& commands, ActionManager* action_manager, + ServiceList* service_list) { TemporaryFile tf; ASSERT_TRUE(tf.fd != -1); ASSERT_TRUE(android::base::WriteStringToFd(init_script, tf.fd)); - TestInit(tf.path, test_function_map, commands, service_list); + TestInit(tf.path, test_function_map, commands, action_manager, service_list); } TEST(init, SimpleEventTrigger) { @@ -91,8 +91,9 @@ pass_test ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); }; std::vector commands{trigger_boot}; + ActionManager action_manager; ServiceList service_list; - TestInitText(init_script, test_function_map, commands, &service_list); + TestInitText(init_script, test_function_map, commands, &action_manager, &service_list); EXPECT_TRUE(expect_true); } @@ -154,8 +155,9 @@ execute_third ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); }; std::vector commands{trigger_boot}; + ActionManager action_manager; ServiceList service_list; - TestInitText(init_script, test_function_map, commands, &service_list); + TestInitText(init_script, test_function_map, commands, &action_manager, &service_list); EXPECT_EQ(3, num_executed); } @@ -170,8 +172,9 @@ service A something )init"; + ActionManager action_manager; ServiceList service_list; - TestInitText(init_script, BuiltinFunctionMap(), {}, &service_list); + TestInitText(init_script, BuiltinFunctionMap(), {}, &action_manager, &service_list); ASSERT_EQ(1, std::distance(service_list.begin(), service_list.end())); auto service = service_list.begin()->get(); @@ -237,13 +240,100 @@ TEST(init, EventTriggerOrderMultipleFiles) { ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); }; std::vector commands{trigger_boot}; + ActionManager action_manager; ServiceList service_list; - - TestInit(start.path, test_function_map, commands, &service_list); + TestInit(start.path, test_function_map, commands, &action_manager, &service_list); EXPECT_EQ(6, num_executed); } +BuiltinFunctionMap GetTestFunctionMapForLazyLoad(int& num_executed, ActionManager& action_manager) { + auto execute_command = [&num_executed](const BuiltinArguments& args) { + EXPECT_EQ(2U, args.size()); + EXPECT_EQ(++num_executed, std::stoi(args[1])); + return Result{}; + }; + auto load_command = [&action_manager](const BuiltinArguments& args) -> Result { + EXPECT_EQ(2U, args.size()); + Parser parser; + parser.AddSectionParser("on", std::make_unique(&action_manager, nullptr)); + if (!parser.ParseConfig(args[1])) { + return Error() << "Failed to load"; + } + return Result{}; + }; + auto trigger_command = [&action_manager](const BuiltinArguments& args) { + EXPECT_EQ(2U, args.size()); + LOG(INFO) << "Queue event trigger: " << args[1]; + action_manager.QueueEventTrigger(args[1]); + return Result{}; + }; + BuiltinFunctionMap test_function_map = { + {"execute", {1, 1, {false, execute_command}}}, + {"load", {1, 1, {false, load_command}}}, + {"trigger", {1, 1, {false, trigger_command}}}, + }; + return test_function_map; +} + +TEST(init, LazilyLoadedActionsCantBeTriggeredByTheSameTrigger) { + // "start" script loads "lazy" script. Even though "lazy" scripts + // defines "on boot" action, it's not executed by the current "boot" + // event because it's already processed. + TemporaryFile lazy; + ASSERT_TRUE(lazy.fd != -1); + ASSERT_TRUE(android::base::WriteStringToFd("on boot\nexecute 2", lazy.fd)); + + TemporaryFile start; + // clang-format off + std::string start_script = "on boot\n" + "load " + std::string(lazy.path) + "\n" + "execute 1"; + // clang-format on + ASSERT_TRUE(android::base::WriteStringToFd(start_script, start.fd)); + + int num_executed = 0; + ActionManager action_manager; + ServiceList service_list; + BuiltinFunctionMap test_function_map = + GetTestFunctionMapForLazyLoad(num_executed, action_manager); + + ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); }; + std::vector commands{trigger_boot}; + TestInit(start.path, test_function_map, commands, &action_manager, &service_list); + + EXPECT_EQ(1, num_executed); +} + +TEST(init, LazilyLoadedActionsCanBeTriggeredByTheNextTrigger) { + // "start" script loads "lazy" script and then triggers "next" event + // which executes "on next" action loaded by the previous command. + TemporaryFile lazy; + ASSERT_TRUE(lazy.fd != -1); + ASSERT_TRUE(android::base::WriteStringToFd("on next\nexecute 2", lazy.fd)); + + TemporaryFile start; + // clang-format off + std::string start_script = "on boot\n" + "load " + std::string(lazy.path) + "\n" + "execute 1\n" + "trigger next"; + // clang-format on + ASSERT_TRUE(android::base::WriteStringToFd(start_script, start.fd)); + + int num_executed = 0; + ActionManager action_manager; + ServiceList service_list; + BuiltinFunctionMap test_function_map = + GetTestFunctionMapForLazyLoad(num_executed, action_manager); + + ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); }; + std::vector commands{trigger_boot}; + TestInit(start.path, test_function_map, commands, &action_manager, &service_list); + + EXPECT_EQ(2, num_executed); +} + TEST(init, RejectsCriticalAndOneshotService) { if (GetIntProperty("ro.product.first_api_level", 10000) < 30) { GTEST_SKIP() << "Test only valid for devices launching with R or later"; diff --git a/init/reboot.cpp b/init/reboot.cpp index 41cf748d8..4e4bfd875 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -892,7 +892,16 @@ static Result DoUserspaceReboot() { sub_reason = "ns_switch"; return Error() << "Failed to switch to bootstrap namespace"; } - // Remove services that were defined in an APEX. + ActionManager::GetInstance().RemoveActionIf([](const auto& action) -> bool { + if (action->IsFromApex()) { + std::string trigger_name = action->BuildTriggersString(); + LOG(INFO) << "Removing action (" << trigger_name << ") from (" << action->filename() + << ":" << action->line() << ")"; + return true; + } + return false; + }); + // Remove services that were defined in an APEX ServiceList::GetInstance().RemoveServiceIf([](const std::unique_ptr& s) -> bool { if (s->is_from_apex()) { LOG(INFO) << "Removing service '" << s->name() << "' because it's defined in an APEX"; From 1eb3394e9c9ea0d19abf195a60ccbe1c28dd8388 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Tue, 10 May 2022 03:37:18 +0900 Subject: [PATCH 2/3] add apex-ready event after post-fs-data Since apexd.status=ready is system-only property, we need a similar or equivalent event or property which non-system APEXes can use to define 'on' trigger actions. Note that services can be started without its own trigger actions by setting 'class'. For example, 'hal'-class services are started 'on boot' automatically. Bug: 202731768 Test: atest CtsInitTestCases Test: atest CtsBluetoothTestCases (cuttlefish's bt apex defines 'on' actions in the APEX config) Change-Id: I6eb62ba8d6e350add2ebafe7da06fcaa57d825ff --- rootdir/init.rc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rootdir/init.rc b/rootdir/init.rc index 8d8da5eda..975fcc0ae 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -538,6 +538,10 @@ on late-init # /data, which in turn can only be loaded when system properties are present. trigger post-fs-data + # APEXes are ready to use. apex-ready is a public trigger similar to apexd.status=ready which + # is a system-private property. + trigger apex-ready + # Should be before netd, but after apex, properties and logging is available. trigger load_bpf_programs @@ -1302,6 +1306,7 @@ on userspace-reboot-fs-remount on userspace-reboot-resume trigger userspace-reboot-fs-remount trigger post-fs-data + trigger apex-ready trigger zygote-start trigger early-boot trigger boot From 38e8e74550fa852fd2dadab415cbea6b4acc29bf Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Tue, 10 May 2022 05:35:35 +0900 Subject: [PATCH 3/3] Use subcontext for APEX configs from /{vendor, odm} Instead of using config file path, use APEX's preinstalled path to determine whether to use subcontext or not for APEX configs. Bug: 232021354 Test: CtsInitTestCases, CtsBluetoothTestCases Change-Id: Iba603f09602f0bec3113e2be3d15c62055c09e72 --- init/init.cpp | 58 +++++++++++++++++++++++++++++++++++++++++---- init/subcontext.cpp | 13 +++++++++- init/subcontext.h | 4 +++- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/init/init.cpp b/init/init.cpp index fd8ee0fe8..4955bc5f2 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -85,6 +85,10 @@ #include "system/core/init/property_service.pb.h" #include "util.h" +#ifndef RECOVERY +#include "com_android_apex.h" +#endif // RECOVERY + using namespace std::chrono_literals; using namespace std::string_literals; @@ -293,14 +297,58 @@ Parser CreateParser(ActionManager& action_manager, ServiceList& service_list) { return parser; } +#ifndef RECOVERY +template +struct LibXmlErrorHandler { + T handler_; + template + LibXmlErrorHandler(Handler&& handler) : handler_(std::move(handler)) { + xmlSetGenericErrorFunc(nullptr, &ErrorHandler); + } + ~LibXmlErrorHandler() { xmlSetGenericErrorFunc(nullptr, nullptr); } + static void ErrorHandler(void*, const char* msg, ...) { + va_list args; + va_start(args, msg); + char* formatted; + if (vasprintf(&formatted, msg, args) >= 0) { + LOG(ERROR) << formatted; + } + free(formatted); + va_end(args); + } +}; + +template +LibXmlErrorHandler(Handler&&) -> LibXmlErrorHandler; +#endif // RECOVERY + // Returns a Parser that accepts scripts from APEX modules. It supports `service` and `on`. Parser CreateApexConfigParser(ActionManager& action_manager, ServiceList& service_list) { Parser parser; - - parser.AddSectionParser( - "service", std::make_unique(&service_list, GetSubcontext(), std::nullopt, - /*from_apex=*/true)); - parser.AddSectionParser("on", std::make_unique(&action_manager, GetSubcontext())); + auto subcontext = GetSubcontext(); +#ifndef RECOVERY + if (subcontext) { + const auto apex_info_list_file = "/apex/apex-info-list.xml"; + auto error_handler = LibXmlErrorHandler([&](const auto& error_message) { + LOG(ERROR) << "Failed to read " << apex_info_list_file << ":" << error_message; + }); + const auto apex_info_list = com::android::apex::readApexInfoList(apex_info_list_file); + if (apex_info_list.has_value()) { + std::vector subcontext_apexes; + for (const auto& info : apex_info_list->getApexInfo()) { + if (info.hasPreinstalledModulePath() && + subcontext->PathMatchesSubcontext(info.getPreinstalledModulePath())) { + subcontext_apexes.push_back(info.getModuleName()); + } + } + subcontext->SetApexList(std::move(subcontext_apexes)); + } + } +#endif // RECOVERY + parser.AddSectionParser("service", + std::make_unique(&service_list, subcontext, std::nullopt, + /*from_apex=*/true)); + parser.AddSectionParser("on", std::make_unique(&action_manager, subcontext)); return parser; } diff --git a/init/subcontext.cpp b/init/subcontext.cpp index 7aa4a9d2c..bb3967e09 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -250,7 +250,14 @@ void Subcontext::Restart() { Fork(); } -bool Subcontext::PathMatchesSubcontext(const std::string& path) { +bool Subcontext::PathMatchesSubcontext(const std::string& path) const { + static const std::string kApexDir = "/apex/"; + if (StartsWith(path, kApexDir)) { + auto begin = kApexDir.size(); + auto end = path.find('/', begin); + auto apex_name = path.substr(begin, end - begin); + return std::find(apex_list_.begin(), apex_list_.end(), apex_name) != apex_list_.end(); + } for (const auto& prefix : path_prefixes_) { if (StartsWith(path, prefix)) { return true; @@ -259,6 +266,10 @@ bool Subcontext::PathMatchesSubcontext(const std::string& path) { return false; } +void Subcontext::SetApexList(std::vector&& apex_list) { + apex_list_ = std::move(apex_list); +} + Result Subcontext::TransmitMessage(const SubcontextCommand& subcontext_command) { if (auto result = SendMessage(socket_, subcontext_command); !result.ok()) { Restart(); diff --git a/init/subcontext.h b/init/subcontext.h index cb4138e69..8acc032d6 100644 --- a/init/subcontext.h +++ b/init/subcontext.h @@ -46,7 +46,8 @@ class Subcontext { Result Execute(const std::vector& args); Result> ExpandArgs(const std::vector& args); void Restart(); - bool PathMatchesSubcontext(const std::string& path); + bool PathMatchesSubcontext(const std::string& path) const; + void SetApexList(std::vector&& apex_list); const std::string& context() const { return context_; } pid_t pid() const { return pid_; } @@ -56,6 +57,7 @@ class Subcontext { Result TransmitMessage(const SubcontextCommand& subcontext_command); std::vector path_prefixes_; + std::vector apex_list_; std::string context_; pid_t pid_; android::base::unique_fd socket_;