Skip to content

Commit 4772f1d

Browse files
author
Tom Cherry
committed
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
1 parent c5cf85d commit 4772f1d

16 files changed

+441
-91
lines changed

‎init/Android.bp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,10 @@ cc_benchmark {
227227

228228
genrule {
229229
name: "generated_stub_builtin_function_map",
230+
tool_files: ["host_builtin_map.py"],
230231
out: ["generated_stub_builtin_function_map.h"],
231-
srcs: ["builtins.cpp"],
232-
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)",
232+
srcs: ["builtins.cpp", "check_builtins.cpp"],
233+
cmd: "$(location host_builtin_map.py) --builtins $(location builtins.cpp) --check_builtins $(location check_builtins.cpp) > $(out)",
233234
}
234235

235236
cc_binary {
@@ -260,6 +261,7 @@ cc_binary {
260261
"action_manager.cpp",
261262
"action_parser.cpp",
262263
"capabilities.cpp",
264+
"check_builtins.cpp",
263265
"epoll.cpp",
264266
"keychords.cpp",
265267
"import_parser.cpp",

‎init/action.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,30 @@ Result<void> Command::InvokeFunc(Subcontext* subcontext) const {
6868
return RunBuiltinFunction(func_, args_, kInitContext);
6969
}
7070

71+
Result<void> Command::CheckCommand() const {
72+
auto builtin_arguments = BuiltinArguments("host_init_verifier");
73+
74+
builtin_arguments.args.resize(args_.size());
75+
builtin_arguments.args[0] = args_[0];
76+
for (size_t i = 1; i < args_.size(); ++i) {
77+
auto expanded_arg = ExpandProps(args_[i]);
78+
if (!expanded_arg) {
79+
if (expanded_arg.error().message().find("doesn't exist while expanding") !=
80+
std::string::npos) {
81+
// If we failed because we won't have a property, use an empty string, which is
82+
// never returned from the parser, to indicate that this field cannot be checked.
83+
builtin_arguments.args[i] = "";
84+
} else {
85+
return expanded_arg.error();
86+
}
87+
} else {
88+
builtin_arguments.args[i] = std::move(*expanded_arg);
89+
}
90+
}
91+
92+
return func_(builtin_arguments);
93+
}
94+
7195
std::string Command::BuildCommandString() const {
7296
return Join(args_, ' ');
7397
}
@@ -107,6 +131,18 @@ std::size_t Action::NumCommands() const {
107131
return commands_.size();
108132
}
109133

134+
size_t Action::CheckAllCommands() const {
135+
size_t failures = 0;
136+
for (const auto& command : commands_) {
137+
if (auto result = command.CheckCommand(); !result) {
138+
LOG(ERROR) << "Command '" << command.BuildCommandString() << "' (" << filename_ << ":"
139+
<< command.line() << ") failed: " << result.error();
140+
++failures;
141+
}
142+
}
143+
return failures;
144+
}
145+
110146
void Action::ExecuteOneCommand(std::size_t command) const {
111147
// We need a copy here since some Command execution may result in
112148
// changing commands_ vector by importing .rc files through parser

‎init/action.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
#ifndef _INIT_ACTION_H
18-
#define _INIT_ACTION_H
17+
#pragma once
1918

2019
#include <map>
2120
#include <queue>
@@ -41,6 +40,7 @@ class Command {
4140

4241
Result<void> InvokeFunc(Subcontext* subcontext) const;
4342
std::string BuildCommandString() const;
43+
Result<void> CheckCommand() const;
4444

4545
int line() const { return line_; }
4646

@@ -63,14 +63,15 @@ class Action {
6363

6464
Result<void> AddCommand(std::vector<std::string>&& args, int line);
6565
void AddCommand(BuiltinFunction f, std::vector<std::string>&& args, int line);
66-
std::size_t NumCommands() const;
66+
size_t NumCommands() const;
6767
void ExecuteOneCommand(std::size_t command) const;
6868
void ExecuteAllCommands() const;
6969
bool CheckEvent(const EventTrigger& event_trigger) const;
7070
bool CheckEvent(const PropertyChange& property_change) const;
7171
bool CheckEvent(const BuiltinAction& builtin_action) const;
7272
std::string BuildTriggersString() const;
7373
void DumpState() const;
74+
size_t CheckAllCommands() const;
7475

7576
bool oneshot() const { return oneshot_; }
7677
const std::string& filename() const { return filename_; }
@@ -96,5 +97,3 @@ class Action {
9697

9798
} // namespace init
9899
} // namespace android
99-
100-
#endif

‎init/action_manager.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ namespace init {
2323

2424
ActionManager::ActionManager() : current_command_(0) {}
2525

26+
size_t ActionManager::CheckAllCommands() {
27+
size_t failures = 0;
28+
for (const auto& action : actions_) {
29+
failures += action->CheckAllCommands();
30+
}
31+
return failures;
32+
}
33+
2634
ActionManager& ActionManager::GetInstance() {
2735
static ActionManager instance;
2836
return instance;

‎init/action_manager.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
#ifndef _INIT_ACTION_MANAGER_H
18-
#define _INIT_ACTION_MANAGER_H
17+
#pragma once
1918

2019
#include <string>
2120
#include <vector>
@@ -32,6 +31,7 @@ class ActionManager {
3231

3332
// Exposed for testing
3433
ActionManager();
34+
size_t CheckAllCommands();
3535

3636
void AddAction(std::unique_ptr<Action> action);
3737
void QueueEventTrigger(const std::string& trigger);
@@ -55,5 +55,3 @@ class ActionManager {
5555

5656
} // namespace init
5757
} // namespace android
58-
59-
#endif

‎init/builtins.cpp

Lines changed: 20 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -201,26 +201,26 @@ static Result<void> do_enable(const BuiltinArguments& args) {
201201
static Result<void> do_exec(const BuiltinArguments& args) {
202202
auto service = Service::MakeTemporaryOneshotService(args.args);
203203
if (!service) {
204-
return Error() << "Could not create exec service";
204+
return Error() << "Could not create exec service: " << service.error();
205205
}
206-
if (auto result = service->ExecStart(); !result) {
206+
if (auto result = (*service)->ExecStart(); !result) {
207207
return Error() << "Could not start exec service: " << result.error();
208208
}
209209

210-
ServiceList::GetInstance().AddService(std::move(service));
210+
ServiceList::GetInstance().AddService(std::move(*service));
211211
return {};
212212
}
213213

214214
static Result<void> do_exec_background(const BuiltinArguments& args) {
215215
auto service = Service::MakeTemporaryOneshotService(args.args);
216216
if (!service) {
217-
return Error() << "Could not create exec background service";
217+
return Error() << "Could not create exec background service: " << service.error();
218218
}
219-
if (auto result = service->Start(); !result) {
219+
if (auto result = (*service)->Start(); !result) {
220220
return Error() << "Could not start exec background service: " << result.error();
221221
}
222222

223-
ServiceList::GetInstance().AddService(std::move(service));
223+
ServiceList::GetInstance().AddService(std::move(*service));
224224
return {};
225225
}
226226

@@ -344,7 +344,7 @@ static Result<void> do_mkdir(const BuiltinArguments& args) {
344344
if (args.size() == 5) {
345345
gid = DecodeUid(args[4]);
346346
if (!gid) {
347-
return Error() << "Unable to decode GID for '" << args[3] << "': " << gid.error();
347+
return Error() << "Unable to decode GID for '" << args[4] << "': " << gid.error();
348348
}
349349
}
350350

@@ -936,40 +936,17 @@ static Result<void> do_chmod(const BuiltinArguments& args) {
936936
}
937937

938938
static Result<void> do_restorecon(const BuiltinArguments& args) {
939-
int ret = 0;
940-
941-
struct flag_type {const char* name; int value;};
942-
static const flag_type flags[] = {
943-
{"--recursive", SELINUX_ANDROID_RESTORECON_RECURSE},
944-
{"--skip-ce", SELINUX_ANDROID_RESTORECON_SKIPCE},
945-
{"--cross-filesystems", SELINUX_ANDROID_RESTORECON_CROSS_FILESYSTEMS},
946-
{0, 0}
947-
};
939+
auto restorecon_info = ParseRestorecon(args.args);
940+
if (!restorecon_info) {
941+
return restorecon_info.error();
942+
}
948943

949-
int flag = 0;
944+
const auto& [flag, paths] = *restorecon_info;
950945

951-
bool in_flags = true;
952-
for (size_t i = 1; i < args.size(); ++i) {
953-
if (android::base::StartsWith(args[i], "--")) {
954-
if (!in_flags) {
955-
return Error() << "flags must precede paths";
956-
}
957-
bool found = false;
958-
for (size_t j = 0; flags[j].name; ++j) {
959-
if (args[i] == flags[j].name) {
960-
flag |= flags[j].value;
961-
found = true;
962-
break;
963-
}
964-
}
965-
if (!found) {
966-
return Error() << "bad flag " << args[i];
967-
}
968-
} else {
969-
in_flags = false;
970-
if (selinux_android_restorecon(args[i].c_str(), flag) < 0) {
971-
ret = errno;
972-
}
946+
int ret = 0;
947+
for (const auto& path : paths) {
948+
if (selinux_android_restorecon(path.c_str(), flag) < 0) {
949+
ret = errno;
973950
}
974951
}
975952

@@ -1056,9 +1033,9 @@ static Result<void> ExecWithRebootOnFailure(const std::string& reboot_reason,
10561033
const BuiltinArguments& args) {
10571034
auto service = Service::MakeTemporaryOneshotService(args.args);
10581035
if (!service) {
1059-
return Error() << "Could not create exec service";
1036+
return Error() << "Could not create exec service: " << service.error();
10601037
}
1061-
service->AddReapCallback([reboot_reason](const siginfo_t& siginfo) {
1038+
(*service)->AddReapCallback([reboot_reason](const siginfo_t& siginfo) {
10621039
if (siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) {
10631040
// TODO (b/122850122): support this in gsi
10641041
if (fscrypt_is_native() && !android::gsi::IsGsiRunning()) {
@@ -1073,10 +1050,10 @@ static Result<void> ExecWithRebootOnFailure(const std::string& reboot_reason,
10731050
}
10741051
}
10751052
});
1076-
if (auto result = service->ExecStart(); !result) {
1053+
if (auto result = (*service)->ExecStart(); !result) {
10771054
return Error() << "Could not start exec service: " << result.error();
10781055
}
1079-
ServiceList::GetInstance().AddService(std::move(service));
1056+
ServiceList::GetInstance().AddService(std::move(*service));
10801057
return {};
10811058
}
10821059

0 commit comments

Comments
 (0)