From c5cf85db23ac8c8a0206e911d73ef1c42ad66ed6 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 31 Jul 2019 13:59:15 -0700 Subject: [PATCH 1/2] init: don't log in expand_props directly It's better to pass the error message to the caller to determine how best to print the error. Test: build Change-Id: Id8857c459df2f26c031650166609608d20e4d051 --- init/action.cpp | 6 ++++-- init/host_init_stubs.h | 1 + init/import_parser.cpp | 11 +++++------ init/property_service.cpp | 9 +++++---- init/selinux.cpp | 34 ++++++++++++++++++-------------- init/service.cpp | 6 ++++-- init/service_parser.cpp | 14 +++++++------ init/subcontext.cpp | 8 ++++---- init/subcontext_test.cpp | 3 ++- init/util.cpp | 41 ++++++++++++++++++++++----------------- init/util.h | 2 +- 11 files changed, 76 insertions(+), 59 deletions(-) diff --git a/init/action.cpp b/init/action.cpp index 0c476df21..97b1c1a97 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -35,9 +35,11 @@ Result RunBuiltinFunction(const BuiltinFunction& function, builtin_arguments.args.resize(args.size()); builtin_arguments.args[0] = args[0]; for (std::size_t i = 1; i < args.size(); ++i) { - if (!expand_props(args[i], &builtin_arguments.args[i])) { - return Error() << "cannot expand '" << args[i] << "'"; + auto expanded_arg = ExpandProps(args[i]); + if (!expanded_arg) { + return expanded_arg.error(); } + builtin_arguments.args[i] = std::move(*expanded_arg); } return function(builtin_arguments); diff --git a/init/host_init_stubs.h b/init/host_init_stubs.h index 7c0544ab1..f9a08a543 100644 --- a/init/host_init_stubs.h +++ b/init/host_init_stubs.h @@ -26,6 +26,7 @@ // android/api-level.h #define __ANDROID_API_P__ 28 +#define __ANDROID_API_R__ 30 // sys/system_properties.h #define PROP_VALUE_MAX 92 diff --git a/init/import_parser.cpp b/init/import_parser.cpp index c72b7d65e..1a43508d7 100644 --- a/init/import_parser.cpp +++ b/init/import_parser.cpp @@ -29,15 +29,14 @@ Result ImportParser::ParseSection(std::vector&& args, return Error() << "single argument needed for import\n"; } - std::string conf_file; - bool ret = expand_props(args[1], &conf_file); - if (!ret) { - return Error() << "error while expanding import"; + auto conf_file = ExpandProps(args[1]); + if (!conf_file) { + return Error() << "Could not expand import: " << conf_file.error(); } - LOG(INFO) << "Added '" << conf_file << "' to import list"; + LOG(INFO) << "Added '" << *conf_file << "' to import list"; if (filename_.empty()) filename_ = filename; - imports_.emplace_back(std::move(conf_file), line); + imports_.emplace_back(std::move(*conf_file), line); return {}; } diff --git a/init/property_service.cpp b/init/property_service.cpp index 37617505d..3819042e9 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -648,13 +648,14 @@ static void LoadProperties(char* data, const char* filter, const char* filename, } std::string raw_filename(fn); - std::string expanded_filename; - if (!expand_props(raw_filename, &expanded_filename)) { - LOG(ERROR) << "Could not expand filename '" << raw_filename << "'"; + auto expanded_filename = ExpandProps(raw_filename); + + if (!expanded_filename) { + LOG(ERROR) << "Could not expand filename ': " << expanded_filename.error(); continue; } - load_properties_from_file(expanded_filename.c_str(), key, properties); + load_properties_from_file(expanded_filename->c_str(), key, properties); } else { value = strchr(key, '='); if (!value) continue; diff --git a/init/selinux.cpp b/init/selinux.cpp index 54be08696..143cdfdae 100644 --- a/init/selinux.cpp +++ b/init/selinux.cpp @@ -497,24 +497,28 @@ void SelinuxSetupKernelLogging() { // This function returns the Android version with which the vendor SEPolicy was compiled. // It is used for version checks such as whether or not vendor_init should be used int SelinuxGetVendorAndroidVersion() { - if (!IsSplitPolicyDevice()) { - // If this device does not split sepolicy files, it's not a Treble device and therefore, - // we assume it's always on the latest platform. - return __ANDROID_API_FUTURE__; - } + static int vendor_android_version = [] { + if (!IsSplitPolicyDevice()) { + // If this device does not split sepolicy files, it's not a Treble device and therefore, + // we assume it's always on the latest platform. + return __ANDROID_API_FUTURE__; + } - std::string version; - if (!GetVendorMappingVersion(&version)) { - LOG(FATAL) << "Could not read vendor SELinux version"; - } + std::string version; + if (!GetVendorMappingVersion(&version)) { + LOG(FATAL) << "Could not read vendor SELinux version"; + } - int major_version; - std::string major_version_str(version, 0, version.find('.')); - if (!ParseInt(major_version_str, &major_version)) { - PLOG(FATAL) << "Failed to parse the vendor sepolicy major version " << major_version_str; - } + int major_version; + std::string major_version_str(version, 0, version.find('.')); + if (!ParseInt(major_version_str, &major_version)) { + PLOG(FATAL) << "Failed to parse the vendor sepolicy major version " + << major_version_str; + } - return major_version; + return major_version; + }(); + return vendor_android_version; } // This function initializes SELinux then execs init to run in the init SELinux context. diff --git a/init/service.cpp b/init/service.cpp index 47f4db94e..380c93bfe 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -100,9 +100,11 @@ static bool ExpandArgsAndExecv(const std::vector& args, bool sigsto expanded_args.resize(args.size()); c_strings.push_back(const_cast(args[0].data())); for (std::size_t i = 1; i < args.size(); ++i) { - if (!expand_props(args[i], &expanded_args[i])) { - LOG(FATAL) << args[0] << ": cannot expand '" << args[i] << "'"; + auto expanded_arg = ExpandProps(args[i]); + if (!expanded_arg) { + LOG(FATAL) << args[0] << ": cannot expand arguments': " << expanded_arg.error(); } + expanded_args[i] = *expanded_arg; c_strings.push_back(expanded_args[i].data()); } c_strings.push_back(nullptr); diff --git a/init/service_parser.cpp b/init/service_parser.cpp index 65d96c665..e45e80415 100644 --- a/init/service_parser.cpp +++ b/init/service_parser.cpp @@ -193,9 +193,9 @@ Result ServiceParser::ParseIoprio(std::vector&& args) { Result ServiceParser::ParseKeycodes(std::vector&& args) { auto it = args.begin() + 1; if (args.size() == 2 && StartsWith(args[1], "$")) { - std::string expanded; - if (!expand_props(args[1], &expanded)) { - return Error() << "Could not expand property '" << args[1] << "'"; + auto expanded = ExpandProps(args[1]); + if (!expanded) { + return expanded.error(); } // If the property is not set, it defaults to none, in which case there are no keycodes @@ -204,7 +204,7 @@ Result ServiceParser::ParseKeycodes(std::vector&& args) { return {}; } - args = Split(expanded, ","); + args = Split(*expanded, ","); it = args.begin(); } @@ -422,9 +422,11 @@ Result ServiceParser::ParseFile(std::vector&& args) { FileDescriptor file; file.type = args[2]; - if (!expand_props(args[1], &file.name)) { - return Error() << "Could not expand property in file path '" << args[1] << "'"; + auto file_name = ExpandProps(args[1]); + if (!file_name) { + return Error() << "Could not expand file path ': " << file_name.error(); } + file.name = *file_name; if (file.name[0] != '/' || file.name.find("../") != std::string::npos) { return Error() << "file name must not be relative"; } diff --git a/init/subcontext.cpp b/init/subcontext.cpp index a13f0c766..00f91d830 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -151,15 +151,15 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec void SubcontextProcess::ExpandArgs(const SubcontextCommand::ExpandArgsCommand& expand_args_command, SubcontextReply* reply) const { for (const auto& arg : expand_args_command.args()) { - auto expanded_prop = std::string{}; - if (!expand_props(arg, &expanded_prop)) { + auto expanded_arg = ExpandProps(arg); + if (!expanded_arg) { auto* failure = reply->mutable_failure(); - failure->set_error_string("Failed to expand '" + arg + "'"); + failure->set_error_string(expanded_arg.error().message()); failure->set_error_errno(0); return; } else { auto* expand_args_reply = reply->mutable_expand_args_reply(); - expand_args_reply->add_expanded_args(expanded_prop); + expand_args_reply->add_expanded_args(*expanded_arg); } } } diff --git a/init/subcontext_test.cpp b/init/subcontext_test.cpp index e120a6272..ae89c389c 100644 --- a/init/subcontext_test.cpp +++ b/init/subcontext_test.cpp @@ -166,7 +166,8 @@ TEST(subcontext, ExpandArgsFailure) { }; auto result = subcontext.ExpandArgs(args); ASSERT_FALSE(result); - EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error().message()); + EXPECT_EQ("unexpected end of string in '" + args[1] + "', looking for }", + result.error().message()); }); } diff --git a/init/util.cpp b/init/util.cpp index 8bfb75598..169f086e0 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -41,8 +41,11 @@ #include #if defined(__ANDROID__) +#include + #include "reboot_utils.h" #include "selabel.h" +#include "selinux.h" #else #include "host_init_stubs.h" #endif @@ -267,12 +270,10 @@ bool is_dir(const char* pathname) { return S_ISDIR(info.st_mode); } -bool expand_props(const std::string& src, std::string* dst) { +Result ExpandProps(const std::string& src) { const char* src_ptr = src.c_str(); - if (!dst) { - return false; - } + std::string dst; /* - variables can either be $x.y or ${x.y}, in case they are only part * of the string. @@ -286,19 +287,19 @@ bool expand_props(const std::string& src, std::string* dst) { c = strchr(src_ptr, '$'); if (!c) { - dst->append(src_ptr); - return true; + dst.append(src_ptr); + return dst; } - dst->append(src_ptr, c); + dst.append(src_ptr, c); c++; if (*c == '$') { - dst->push_back(*(c++)); + dst.push_back(*(c++)); src_ptr = c; continue; } else if (*c == '\0') { - return true; + return dst; } std::string prop_name; @@ -308,8 +309,7 @@ bool expand_props(const std::string& src, std::string* dst) { const char* end = strchr(c, '}'); if (!end) { // failed to find closing brace, abort. - LOG(ERROR) << "unexpected end of string in '" << src << "', looking for }"; - return false; + return Error() << "unexpected end of string in '" << src << "', looking for }"; } prop_name = std::string(c, end); c = end + 1; @@ -320,29 +320,34 @@ bool expand_props(const std::string& src, std::string* dst) { } } else { prop_name = c; - LOG(ERROR) << "using deprecated syntax for specifying property '" << c << "', use ${name} instead"; + if (SelinuxGetVendorAndroidVersion() >= __ANDROID_API_R__) { + return Error() << "using deprecated syntax for specifying property '" << c + << "', use ${name} instead"; + } else { + LOG(ERROR) << "using deprecated syntax for specifying property '" << c + << "', use ${name} instead"; + } c += prop_name.size(); } if (prop_name.empty()) { - LOG(ERROR) << "invalid zero-length property name in '" << src << "'"; - return false; + return Error() << "invalid zero-length property name in '" << src << "'"; } std::string prop_val = android::base::GetProperty(prop_name, ""); if (prop_val.empty()) { if (def_val.empty()) { - LOG(ERROR) << "property '" << prop_name << "' doesn't exist while expanding '" << src << "'"; - return false; + return Error() << "property '" << prop_name << "' doesn't exist while expanding '" + << src << "'"; } prop_val = def_val; } - dst->append(prop_val); + dst.append(prop_val); src_ptr = c; } - return true; + return dst; } static std::string init_android_dt_dir() { diff --git a/init/util.h b/init/util.h index 6a12fb6f1..98de1f2bb 100644 --- a/init/util.h +++ b/init/util.h @@ -52,7 +52,7 @@ void import_kernel_cmdline(bool in_qemu, const std::function&); bool make_dir(const std::string& path, mode_t mode); bool is_dir(const char* pathname); -bool expand_props(const std::string& src, std::string* dst); +Result ExpandProps(const std::string& src); // Returns the platform's Android DT directory as specified in the kernel cmdline. // If the platform does not configure a custom DT path, returns the standard one (based in procfs). From 4772f1da47bfdad8e0b3e3cdbf1bdc6bda0443b1 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 30 Jul 2019 09:34:41 -0700 Subject: [PATCH 2/2] init: check the arguments of builtins during the build Host init verifier already checks that the names and number of arguments for builtins are correct, but it can check more. This change ensures that property expansions are well formed, and that arguments that can be parsed on the host are correct. For example it checks that UIDs and GIDs exist, that numerical values can be parsed, and that rlimit strings are correct. Test: build Change-Id: Ied8882498a88a9f8324db6b8d1020aeeccc8177b --- init/Android.bp | 6 +- init/action.cpp | 36 +++++++ init/action.h | 9 +- init/action_manager.cpp | 8 ++ init/action_manager.h | 6 +- init/builtins.cpp | 65 ++++-------- init/check_builtins.cpp | 197 ++++++++++++++++++++++++++++++++++++ init/check_builtins.h | 40 ++++++++ init/host_builtin_map.py | 46 +++++++++ init/host_init_verifier.cpp | 10 +- init/property_service.cpp | 9 +- init/service.cpp | 20 ++-- init/service.h | 3 +- init/service_test.cpp | 13 +-- init/util.cpp | 54 ++++++++++ init/util.h | 12 +-- 16 files changed, 442 insertions(+), 92 deletions(-) create mode 100644 init/check_builtins.cpp create mode 100644 init/check_builtins.h create mode 100755 init/host_builtin_map.py diff --git a/init/Android.bp b/init/Android.bp index ba6008547..31e417369 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -227,9 +227,10 @@ cc_benchmark { genrule { name: "generated_stub_builtin_function_map", + tool_files: ["host_builtin_map.py"], out: ["generated_stub_builtin_function_map.h"], - srcs: ["builtins.cpp"], - cmd: "sed -n '/Builtin-function-map start/{:a;n;/Builtin-function-map end/q;p;ba}' $(in) | sed -e 's/do_[^}]*/do_stub/g' > $(out)", + srcs: ["builtins.cpp", "check_builtins.cpp"], + cmd: "$(location host_builtin_map.py) --builtins $(location builtins.cpp) --check_builtins $(location check_builtins.cpp) > $(out)", } cc_binary { @@ -260,6 +261,7 @@ cc_binary { "action_manager.cpp", "action_parser.cpp", "capabilities.cpp", + "check_builtins.cpp", "epoll.cpp", "keychords.cpp", "import_parser.cpp", diff --git a/init/action.cpp b/init/action.cpp index 97b1c1a97..65ba25d1d 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -68,6 +68,30 @@ Result Command::InvokeFunc(Subcontext* subcontext) const { return RunBuiltinFunction(func_, args_, kInitContext); } +Result Command::CheckCommand() const { + auto builtin_arguments = BuiltinArguments("host_init_verifier"); + + builtin_arguments.args.resize(args_.size()); + builtin_arguments.args[0] = args_[0]; + for (size_t i = 1; i < args_.size(); ++i) { + auto expanded_arg = ExpandProps(args_[i]); + if (!expanded_arg) { + if (expanded_arg.error().message().find("doesn't exist while expanding") != + std::string::npos) { + // If we failed because we won't have a property, use an empty string, which is + // never returned from the parser, to indicate that this field cannot be checked. + builtin_arguments.args[i] = ""; + } else { + return expanded_arg.error(); + } + } else { + builtin_arguments.args[i] = std::move(*expanded_arg); + } + } + + return func_(builtin_arguments); +} + std::string Command::BuildCommandString() const { return Join(args_, ' '); } @@ -107,6 +131,18 @@ std::size_t Action::NumCommands() const { return commands_.size(); } +size_t Action::CheckAllCommands() const { + size_t failures = 0; + for (const auto& command : commands_) { + if (auto result = command.CheckCommand(); !result) { + LOG(ERROR) << "Command '" << command.BuildCommandString() << "' (" << filename_ << ":" + << command.line() << ") failed: " << result.error(); + ++failures; + } + } + return failures; +} + void Action::ExecuteOneCommand(std::size_t command) const { // We need a copy here since some Command execution may result in // changing commands_ vector by importing .rc files through parser diff --git a/init/action.h b/init/action.h index 80c1da43a..1534bf987 100644 --- a/init/action.h +++ b/init/action.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef _INIT_ACTION_H -#define _INIT_ACTION_H +#pragma once #include #include @@ -41,6 +40,7 @@ class Command { Result InvokeFunc(Subcontext* subcontext) const; std::string BuildCommandString() const; + Result CheckCommand() const; int line() const { return line_; } @@ -63,7 +63,7 @@ class Action { Result AddCommand(std::vector&& args, int line); void AddCommand(BuiltinFunction f, std::vector&& args, int line); - std::size_t NumCommands() const; + size_t NumCommands() const; void ExecuteOneCommand(std::size_t command) const; void ExecuteAllCommands() const; bool CheckEvent(const EventTrigger& event_trigger) const; @@ -71,6 +71,7 @@ class Action { bool CheckEvent(const BuiltinAction& builtin_action) const; std::string BuildTriggersString() const; void DumpState() const; + size_t CheckAllCommands() const; bool oneshot() const { return oneshot_; } const std::string& filename() const { return filename_; } @@ -96,5 +97,3 @@ class Action { } // namespace init } // namespace android - -#endif diff --git a/init/action_manager.cpp b/init/action_manager.cpp index 541c8f21d..ebca762ca 100644 --- a/init/action_manager.cpp +++ b/init/action_manager.cpp @@ -23,6 +23,14 @@ namespace init { ActionManager::ActionManager() : current_command_(0) {} +size_t ActionManager::CheckAllCommands() { + size_t failures = 0; + for (const auto& action : actions_) { + failures += action->CheckAllCommands(); + } + return failures; +} + ActionManager& ActionManager::GetInstance() { static ActionManager instance; return instance; diff --git a/init/action_manager.h b/init/action_manager.h index 5f47a6db0..a2b95acad 100644 --- a/init/action_manager.h +++ b/init/action_manager.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef _INIT_ACTION_MANAGER_H -#define _INIT_ACTION_MANAGER_H +#pragma once #include #include @@ -32,6 +31,7 @@ class ActionManager { // Exposed for testing ActionManager(); + size_t CheckAllCommands(); void AddAction(std::unique_ptr action); void QueueEventTrigger(const std::string& trigger); @@ -55,5 +55,3 @@ class ActionManager { } // namespace init } // namespace android - -#endif diff --git a/init/builtins.cpp b/init/builtins.cpp index b6e26a120..e75f5cbd3 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -201,26 +201,26 @@ static Result do_enable(const BuiltinArguments& args) { static Result do_exec(const BuiltinArguments& args) { auto service = Service::MakeTemporaryOneshotService(args.args); if (!service) { - return Error() << "Could not create exec service"; + return Error() << "Could not create exec service: " << service.error(); } - if (auto result = service->ExecStart(); !result) { + if (auto result = (*service)->ExecStart(); !result) { return Error() << "Could not start exec service: " << result.error(); } - ServiceList::GetInstance().AddService(std::move(service)); + ServiceList::GetInstance().AddService(std::move(*service)); return {}; } static Result do_exec_background(const BuiltinArguments& args) { auto service = Service::MakeTemporaryOneshotService(args.args); if (!service) { - return Error() << "Could not create exec background service"; + return Error() << "Could not create exec background service: " << service.error(); } - if (auto result = service->Start(); !result) { + if (auto result = (*service)->Start(); !result) { return Error() << "Could not start exec background service: " << result.error(); } - ServiceList::GetInstance().AddService(std::move(service)); + ServiceList::GetInstance().AddService(std::move(*service)); return {}; } @@ -344,7 +344,7 @@ static Result do_mkdir(const BuiltinArguments& args) { if (args.size() == 5) { gid = DecodeUid(args[4]); if (!gid) { - return Error() << "Unable to decode GID for '" << args[3] << "': " << gid.error(); + return Error() << "Unable to decode GID for '" << args[4] << "': " << gid.error(); } } @@ -936,40 +936,17 @@ static Result do_chmod(const BuiltinArguments& args) { } static Result do_restorecon(const BuiltinArguments& args) { + auto restorecon_info = ParseRestorecon(args.args); + if (!restorecon_info) { + return restorecon_info.error(); + } + + const auto& [flag, paths] = *restorecon_info; + int ret = 0; - - struct flag_type {const char* name; int value;}; - static const flag_type flags[] = { - {"--recursive", SELINUX_ANDROID_RESTORECON_RECURSE}, - {"--skip-ce", SELINUX_ANDROID_RESTORECON_SKIPCE}, - {"--cross-filesystems", SELINUX_ANDROID_RESTORECON_CROSS_FILESYSTEMS}, - {0, 0} - }; - - int flag = 0; - - bool in_flags = true; - for (size_t i = 1; i < args.size(); ++i) { - if (android::base::StartsWith(args[i], "--")) { - if (!in_flags) { - return Error() << "flags must precede paths"; - } - bool found = false; - for (size_t j = 0; flags[j].name; ++j) { - if (args[i] == flags[j].name) { - flag |= flags[j].value; - found = true; - break; - } - } - if (!found) { - return Error() << "bad flag " << args[i]; - } - } else { - in_flags = false; - if (selinux_android_restorecon(args[i].c_str(), flag) < 0) { - ret = errno; - } + for (const auto& path : paths) { + if (selinux_android_restorecon(path.c_str(), flag) < 0) { + ret = errno; } } @@ -1056,9 +1033,9 @@ static Result ExecWithRebootOnFailure(const std::string& reboot_reason, const BuiltinArguments& args) { auto service = Service::MakeTemporaryOneshotService(args.args); if (!service) { - return Error() << "Could not create exec service"; + return Error() << "Could not create exec service: " << service.error(); } - service->AddReapCallback([reboot_reason](const siginfo_t& siginfo) { + (*service)->AddReapCallback([reboot_reason](const siginfo_t& siginfo) { if (siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) { // TODO (b/122850122): support this in gsi if (fscrypt_is_native() && !android::gsi::IsGsiRunning()) { @@ -1073,10 +1050,10 @@ static Result ExecWithRebootOnFailure(const std::string& reboot_reason, } } }); - if (auto result = service->ExecStart(); !result) { + if (auto result = (*service)->ExecStart(); !result) { return Error() << "Could not start exec service: " << result.error(); } - ServiceList::GetInstance().AddService(std::move(service)); + ServiceList::GetInstance().AddService(std::move(*service)); return {}; } diff --git a/init/check_builtins.cpp b/init/check_builtins.cpp new file mode 100644 index 000000000..3bd477470 --- /dev/null +++ b/init/check_builtins.cpp @@ -0,0 +1,197 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Note that these check functions cannot check expanded arguments from properties, since they will +// not know what those properties would be at runtime. They will be passed an empty string in the +// situation that the input line had a property expansion without a default value, since an empty +// string is otherwise an impossible value. They should therefore disregard checking empty +// arguments. + +#include "check_builtins.h" + +#include + +#include +#include +#include + +#include "builtin_arguments.h" +#include "rlimit_parser.h" +#include "service.h" +#include "util.h" + +using android::base::ParseInt; +using android::base::StartsWith; + +#define ReturnIfAnyArgsEmpty() \ + for (const auto& arg : args) { \ + if (arg.empty()) { \ + return {}; \ + } \ + } + +namespace android { +namespace init { + +Result check_chown(const BuiltinArguments& args) { + if (!args[1].empty()) { + auto uid = DecodeUid(args[1]); + if (!uid) { + return Error() << "Unable to decode UID for '" << args[1] << "': " << uid.error(); + } + } + + // GID is optional and pushes the index of path out by one if specified. + if (args.size() == 4 && !args[2].empty()) { + auto gid = DecodeUid(args[2]); + if (!gid) { + return Error() << "Unable to decode GID for '" << args[2] << "': " << gid.error(); + } + } + + return {}; +} + +Result check_exec(const BuiltinArguments& args) { + ReturnIfAnyArgsEmpty(); + + auto result = Service::MakeTemporaryOneshotService(args.args); + if (!result) { + return result.error(); + } + + return {}; +} + +Result check_exec_background(const BuiltinArguments& args) { + return check_exec(std::move(args)); +} + +Result check_load_system_props(const BuiltinArguments& args) { + return Error() << "'load_system_props' is deprecated"; +} + +Result check_loglevel(const BuiltinArguments& args) { + ReturnIfAnyArgsEmpty(); + + int log_level = -1; + ParseInt(args[1], &log_level); + if (log_level < 0 || log_level > 7) { + return Error() << "loglevel must be in the range of 0-7"; + } + return {}; +} + +Result check_mkdir(const BuiltinArguments& args) { + if (args.size() >= 4) { + if (!args[3].empty()) { + auto uid = DecodeUid(args[3]); + if (!uid) { + return Error() << "Unable to decode UID for '" << args[3] << "': " << uid.error(); + } + } + + if (args.size() == 5 && !args[4].empty()) { + auto gid = DecodeUid(args[4]); + if (!gid) { + return Error() << "Unable to decode GID for '" << args[4] << "': " << gid.error(); + } + } + } + + return {}; +} + +Result check_restorecon(const BuiltinArguments& args) { + ReturnIfAnyArgsEmpty(); + + auto restorecon_info = ParseRestorecon(args.args); + if (!restorecon_info) { + return restorecon_info.error(); + } + + return {}; +} + +Result check_restorecon_recursive(const BuiltinArguments& args) { + return check_restorecon(std::move(args)); +} + +Result check_setprop(const BuiltinArguments& args) { + const std::string& name = args[1]; + if (name.empty()) { + return {}; + } + const std::string& value = args[2]; + + if (!IsLegalPropertyName(name)) { + return Error() << "'" << name << "' is not a legal property name"; + } + + if (!value.empty()) { + if (auto result = IsLegalPropertyValue(name, value); !result) { + return result.error(); + } + } + + if (StartsWith(name, "ctl.")) { + return Error() + << "Do not set ctl. properties from init; call the Service functions directly"; + } + + static constexpr const char kRestoreconProperty[] = "selinux.restorecon_recursive"; + if (name == kRestoreconProperty) { + return Error() << "Do not set '" << kRestoreconProperty + << "' from init; use the restorecon builtin directly"; + } + + return {}; +} + +Result check_setrlimit(const BuiltinArguments& args) { + ReturnIfAnyArgsEmpty(); + + auto rlimit = ParseRlimit(args.args); + if (!rlimit) return rlimit.error(); + return {}; +} + +Result check_sysclktz(const BuiltinArguments& args) { + ReturnIfAnyArgsEmpty(); + + struct timezone tz = {}; + if (!android::base::ParseInt(args[1], &tz.tz_minuteswest)) { + return Error() << "Unable to parse mins_west_of_gmt"; + } + return {}; +} + +Result check_wait(const BuiltinArguments& args) { + if (args.size() == 3 && !args[2].empty()) { + int timeout_int; + if (!android::base::ParseInt(args[2], &timeout_int)) { + return Error() << "failed to parse timeout"; + } + } + return {}; +} + +Result check_wait_for_prop(const BuiltinArguments& args) { + return check_setprop(std::move(args)); +} + +} // namespace init +} // namespace android diff --git a/init/check_builtins.h b/init/check_builtins.h new file mode 100644 index 000000000..c974e885d --- /dev/null +++ b/init/check_builtins.h @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "builtin_arguments.h" +#include "result.h" + +namespace android { +namespace init { + +Result check_chown(const BuiltinArguments& args); +Result check_exec(const BuiltinArguments& args); +Result check_exec_background(const BuiltinArguments& args); +Result check_load_system_props(const BuiltinArguments& args); +Result check_loglevel(const BuiltinArguments& args); +Result check_mkdir(const BuiltinArguments& args); +Result check_restorecon(const BuiltinArguments& args); +Result check_restorecon_recursive(const BuiltinArguments& args); +Result check_setprop(const BuiltinArguments& args); +Result check_setrlimit(const BuiltinArguments& args); +Result check_sysclktz(const BuiltinArguments& args); +Result check_wait(const BuiltinArguments& args); +Result check_wait_for_prop(const BuiltinArguments& args); + +} // namespace init +} // namespace android diff --git a/init/host_builtin_map.py b/init/host_builtin_map.py new file mode 100755 index 000000000..6afcb173c --- /dev/null +++ b/init/host_builtin_map.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python +"""Generates the builtins map to be used by host_init_verifier. + +It copies the builtin function map from builtins.cpp, then replaces do_xxx() functions with the +equivalent check_xxx() if found in check_builtins.cpp. + +""" + +import re +import argparse + +parser = argparse.ArgumentParser('host_builtin_map.py') +parser.add_argument('--builtins', required=True, help='Path to builtins.cpp') +parser.add_argument('--check_builtins', required=True, help='Path to check_builtins.cpp') +args = parser.parse_args() + +CHECK_REGEX = re.compile(r'.+check_(\S+)\(.+') +check_functions = [] +with open(args.check_builtins) as check_file: + for line in check_file: + match = CHECK_REGEX.match(line) + if match: + check_functions.append(match.group(1)) + +function_map = [] +with open(args.builtins) as builtins_file: + in_function_map = False + for line in builtins_file: + if '// Builtin-function-map start' in line: + in_function_map = True + elif '// Builtin-function-map end' in line: + in_function_map = False + elif in_function_map: + function_map.append(line) + +DO_REGEX = re.compile(r'.+do_([^\}]+).+') +FUNCTION_REGEX = re.compile(r'(do_[^\}]+)') +for line in function_map: + match = DO_REGEX.match(line) + if match: + if match.group(1) in check_functions: + print line.replace('do_', 'check_'), + else: + print FUNCTION_REGEX.sub('check_stub', line), + else: + print line, diff --git a/init/host_init_verifier.cpp b/init/host_init_verifier.cpp index dfde51b1a..a5a5b1b55 100644 --- a/init/host_init_verifier.cpp +++ b/init/host_init_verifier.cpp @@ -35,6 +35,7 @@ #include "action.h" #include "action_manager.h" #include "action_parser.h" +#include "check_builtins.h" #include "host_import_parser.h" #include "host_init_stubs.h" #include "parser.h" @@ -163,7 +164,7 @@ ReadInterfaceInheritanceHierarchy(const std::string& interface_inheritance_hiera namespace android { namespace init { -static Result do_stub(const BuiltinArguments& args) { +static Result check_stub(const BuiltinArguments& args) { return {}; } @@ -238,9 +239,10 @@ int main(int argc, char** argv) { LOG(ERROR) << "Failed to open init rc script '" << *argv << "'"; return EXIT_FAILURE; } - if (parser.parse_error_count() > 0) { - LOG(ERROR) << "Failed to parse init script '" << *argv << "' with " - << parser.parse_error_count() << " errors"; + size_t failures = parser.parse_error_count() + am.CheckAllCommands(); + if (failures > 0) { + LOG(ERROR) << "Failed to parse init script '" << *argv << "' with " << failures + << " errors"; return EXIT_FAILURE; } return EXIT_SUCCESS; diff --git a/init/property_service.cpp b/init/property_service.cpp index 3819042e9..17622a32b 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -174,13 +174,8 @@ static uint32_t PropertySet(const std::string& name, const std::string& value, s return PROP_ERROR_INVALID_NAME; } - if (valuelen >= PROP_VALUE_MAX && !StartsWith(name, "ro.")) { - *error = "Property value too long"; - return PROP_ERROR_INVALID_VALUE; - } - - if (mbstowcs(nullptr, value.data(), 0) == static_cast(-1)) { - *error = "Value is not a UTF8 encoded string"; + if (auto result = IsLegalPropertyValue(name, value); !result) { + *error = result.error().message(); return PROP_ERROR_INVALID_VALUE; } diff --git a/init/service.cpp b/init/service.cpp index 380c93bfe..e60c20d75 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -635,7 +635,8 @@ void Service::StopOrReset(int how) { } } -std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector& args) { +Result> Service::MakeTemporaryOneshotService( + const std::vector& args) { // Parse the arguments: exec [SECLABEL [UID [GID]*] --] COMMAND ARGS... // SECLABEL can be a - to denote default std::size_t command_arg = 1; @@ -646,13 +647,11 @@ std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector< } } if (command_arg > 4 + NR_SVC_SUPP_GIDS) { - LOG(ERROR) << "exec called with too many supplementary group ids"; - return nullptr; + return Error() << "exec called with too many supplementary group ids"; } if (command_arg >= args.size()) { - LOG(ERROR) << "exec called without command"; - return nullptr; + return Error() << "exec called without command"; } std::vector str_args(args.begin() + command_arg, args.end()); @@ -671,8 +670,7 @@ std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector< if (command_arg > 3) { uid = DecodeUid(args[2]); if (!uid) { - LOG(ERROR) << "Unable to decode UID for '" << args[2] << "': " << uid.error(); - return nullptr; + return Error() << "Unable to decode UID for '" << args[2] << "': " << uid.error(); } } Result gid = 0; @@ -680,16 +678,14 @@ std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector< if (command_arg > 4) { gid = DecodeUid(args[3]); if (!gid) { - LOG(ERROR) << "Unable to decode GID for '" << args[3] << "': " << gid.error(); - return nullptr; + return Error() << "Unable to decode GID for '" << args[3] << "': " << gid.error(); } std::size_t nr_supp_gids = command_arg - 1 /* -- */ - 4 /* exec SECLABEL UID GID */; for (size_t i = 0; i < nr_supp_gids; ++i) { auto supp_gid = DecodeUid(args[4 + i]); if (!supp_gid) { - LOG(ERROR) << "Unable to decode GID for '" << args[4 + i] - << "': " << supp_gid.error(); - return nullptr; + return Error() << "Unable to decode GID for '" << args[4 + i] + << "': " << supp_gid.error(); } supp_gids.push_back(*supp_gid); } diff --git a/init/service.h b/init/service.h index cdf31bbf9..6f79faac7 100644 --- a/init/service.h +++ b/init/service.h @@ -71,7 +71,8 @@ class Service { const std::vector& supp_gids, int namespace_flags, const std::string& seclabel, Subcontext* subcontext_for_restart_commands, const std::vector& args); - static std::unique_ptr MakeTemporaryOneshotService(const std::vector& args); + static Result> MakeTemporaryOneshotService( + const std::vector& args); bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; } Result ExecStart(); diff --git a/init/service_test.cpp b/init/service_test.cpp index 6a34acc55..c9cc7bdf0 100644 --- a/init/service_test.cpp +++ b/init/service_test.cpp @@ -75,15 +75,15 @@ TEST(service, pod_initialized) { TEST(service, make_temporary_oneshot_service_invalid_syntax) { std::vector args; // Nothing. - ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); + ASSERT_FALSE(Service::MakeTemporaryOneshotService(args)); // No arguments to 'exec'. args.push_back("exec"); - ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); + ASSERT_FALSE(Service::MakeTemporaryOneshotService(args)); // No command in "exec --". args.push_back("--"); - ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); + ASSERT_FALSE(Service::MakeTemporaryOneshotService(args)); } TEST(service, make_temporary_oneshot_service_too_many_supplementary_gids) { @@ -97,7 +97,7 @@ TEST(service, make_temporary_oneshot_service_too_many_supplementary_gids) { } args.push_back("--"); args.push_back("/system/bin/id"); - ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); + ASSERT_FALSE(Service::MakeTemporaryOneshotService(args)); } static void Test_make_temporary_oneshot_service(bool dash_dash, bool seclabel, bool uid, bool gid, @@ -122,8 +122,9 @@ static void Test_make_temporary_oneshot_service(bool dash_dash, bool seclabel, b } args.push_back("/system/bin/toybox"); args.push_back("id"); - auto svc = Service::MakeTemporaryOneshotService(args); - ASSERT_NE(nullptr, svc); + auto service_ret = Service::MakeTemporaryOneshotService(args); + ASSERT_TRUE(service_ret); + auto svc = std::move(*service_ret); if (seclabel) { ASSERT_EQ("u:r:su:s0", svc->seclabel()); diff --git a/init/util.cpp b/init/util.cpp index 169f086e0..053237509 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -42,6 +42,7 @@ #if defined(__ANDROID__) #include +#include #include "reboot_utils.h" #include "selabel.h" @@ -51,6 +52,7 @@ #endif using android::base::boot_clock; +using android::base::StartsWith; using namespace std::literals::string_literals; namespace android { @@ -419,6 +421,58 @@ bool IsLegalPropertyName(const std::string& name) { return true; } +Result IsLegalPropertyValue(const std::string& name, const std::string& value) { + if (value.size() >= PROP_VALUE_MAX && !StartsWith(name, "ro.")) { + return Error() << "Property value too long"; + } + + if (mbstowcs(nullptr, value.data(), 0) == static_cast(-1)) { + return Error() << "Value is not a UTF8 encoded string"; + } + + return {}; +} + +Result>> ParseRestorecon( + const std::vector& args) { + struct flag_type { + const char* name; + int value; + }; + static const flag_type flags[] = { + {"--recursive", SELINUX_ANDROID_RESTORECON_RECURSE}, + {"--skip-ce", SELINUX_ANDROID_RESTORECON_SKIPCE}, + {"--cross-filesystems", SELINUX_ANDROID_RESTORECON_CROSS_FILESYSTEMS}, + {0, 0}}; + + int flag = 0; + std::vector paths; + + bool in_flags = true; + for (size_t i = 1; i < args.size(); ++i) { + if (android::base::StartsWith(args[i], "--")) { + if (!in_flags) { + return Error() << "flags must precede paths"; + } + bool found = false; + for (size_t j = 0; flags[j].name; ++j) { + if (args[i] == flags[j].name) { + flag |= flags[j].value; + found = true; + break; + } + } + if (!found) { + return Error() << "bad flag " << args[i]; + } + } else { + in_flags = false; + paths.emplace_back(args[i]); + } + } + return std::pair(flag, paths); +} + static void InitAborter(const char* abort_message) { // When init forks, it continues to use this aborter for LOG(FATAL), but we want children to // simply abort instead of trying to reboot the system. diff --git a/init/util.h b/init/util.h index 98de1f2bb..4cccefe43 100644 --- a/init/util.h +++ b/init/util.h @@ -14,24 +14,20 @@ * limitations under the License. */ -#ifndef _INIT_UTIL_H_ -#define _INIT_UTIL_H_ +#pragma once #include #include #include #include -#include #include #include -#include #include "result.h" using android::base::boot_clock; -using namespace std::chrono_literals; namespace android { namespace init { @@ -62,11 +58,13 @@ bool read_android_dt_file(const std::string& sub_path, std::string* dt_content); bool is_android_dt_value_expected(const std::string& sub_path, const std::string& expected_content); bool IsLegalPropertyName(const std::string& name); +Result IsLegalPropertyValue(const std::string& name, const std::string& value); + +Result>> ParseRestorecon( + const std::vector& args); void SetStdioToDevNull(char** argv); void InitKernelLogging(char** argv); bool IsRecoveryMode(); } // namespace init } // namespace android - -#endif