Skip to content

Commit 18278d2

Browse files
author
Tom Cherry
committed
init: make triggering shutdown from vendor_init better
Previously, we assumed that TriggerShutdown() should never be called from vendor_init and used property service as a back up in case it ever did. We have since then found out that vendor_init may indeed call TriggerShutdown() and we want to make it just as strict as it is in init, wherein it will immediately start the shutdown sequence without executing any further commands. Test: init unit tests, trigger shuttdown from init and vendor_init Change-Id: I1f44dae801a28269eb8127879a8b7d6adff6f353
1 parent e91c76b commit 18278d2

File tree

11 files changed

+50
-24
lines changed

11 files changed

+50
-24
lines changed

init/builtins.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,7 @@ static Result<void> reboot_into_recovery(const std::vector<std::string>& options
140140
if (!write_bootloader_message(options, &err)) {
141141
return Error() << "Failed to set bootloader message: " << err;
142142
}
143-
// This function should only be reached from init and not from vendor_init, and we want to
144-
// immediately trigger reboot instead of relaying through property_service. Older devices may
145-
// still have paths that reach here from vendor_init, so we keep the property_set as a fallback.
146-
if (getpid() == 1) {
147-
TriggerShutdown("reboot,recovery");
148-
} else {
149-
property_set("sys.powerctl", "reboot,recovery");
150-
}
143+
trigger_shutdown("reboot,recovery");
151144
return {};
152145
}
153146

@@ -554,7 +547,7 @@ static Result<void> queue_fs_event(int code, bool userdata_remount) {
554547
// support userdata remount on FDE devices, this should never been triggered. Time to
555548
// panic!
556549
LOG(ERROR) << "Userdata remount is not supported on FDE devices. How did you get here?";
557-
TriggerShutdown("reboot,requested-userdata-remount-on-fde-device");
550+
trigger_shutdown("reboot,requested-userdata-remount-on-fde-device");
558551
}
559552
ActionManager::GetInstance().QueueEventTrigger("encrypt");
560553
return {};
@@ -564,7 +557,7 @@ static Result<void> queue_fs_event(int code, bool userdata_remount) {
564557
// don't support userdata remount on FDE devices, this should never been triggered.
565558
// Time to panic!
566559
LOG(ERROR) << "Userdata remount is not supported on FDE devices. How did you get here?";
567-
TriggerShutdown("reboot,requested-userdata-remount-on-fde-device");
560+
trigger_shutdown("reboot,requested-userdata-remount-on-fde-device");
568561
}
569562
property_set("ro.crypto.state", "encrypted");
570563
property_set("ro.crypto.type", "block");
@@ -1148,7 +1141,7 @@ static Result<void> do_remount_userdata(const BuiltinArguments& args) {
11481141
}
11491142
// TODO(b/135984674): check that fstab contains /data.
11501143
if (auto rc = fs_mgr_remount_userdata_into_checkpointing(&fstab); rc < 0) {
1151-
TriggerShutdown("reboot,mount-userdata-failed");
1144+
trigger_shutdown("reboot,mount-userdata-failed");
11521145
}
11531146
if (auto result = queue_fs_event(initial_mount_fstab_return_code, true); !result) {
11541147
return Error() << "queue_fs_event() failed: " << result.error();

init/host_init_stubs.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,6 @@
3535
namespace android {
3636
namespace init {
3737

38-
// init.h
39-
inline void TriggerShutdown(const std::string&) {
40-
abort();
41-
}
42-
4338
// property_service.h
4439
inline bool CanReadProperty(const std::string&, const std::string&) {
4540
return true;

init/init.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ void ResetWaitForProp() {
180180
waiting_for_prop.reset();
181181
}
182182

183-
void TriggerShutdown(const std::string& command) {
183+
static void TriggerShutdown(const std::string& command) {
184184
// We can't call HandlePowerctlMessage() directly in this function,
185185
// because it modifies the contents of the action queue, which can cause the action queue
186186
// to get into a bad state if this function is called from a command being executed by the
@@ -681,6 +681,8 @@ int SecondStageMain(int argc, char** argv) {
681681

682682
boot_clock::time_point start_time = boot_clock::now();
683683

684+
trigger_shutdown = TriggerShutdown;
685+
684686
SetStdioToDevNull(argv);
685687
InitKernelLogging(argv);
686688
LOG(INFO) << "init second stage started!";

init/init.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ namespace init {
3131
Parser CreateParser(ActionManager& action_manager, ServiceList& service_list);
3232
Parser CreateServiceOnlyParser(ServiceList& service_list);
3333

34-
void TriggerShutdown(const std::string& command);
35-
3634
bool start_waiting_for_property(const char *name, const char *value);
3735

3836
void DumpState();

init/reboot.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ static Result<void> DoUserspaceReboot() {
731731
auto guard = android::base::make_scope_guard([] {
732732
// Leave shutdown so that we can handle a full reboot.
733733
LeaveShutdown();
734-
TriggerShutdown("reboot,abort-userspace-reboot");
734+
trigger_shutdown("reboot,abort-userspace-reboot");
735735
});
736736
// Triggering userspace-reboot-requested will result in a bunch of set_prop
737737
// actions. We should make sure, that all of them are propagated before

init/service.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
#if defined(__ANDROID__)
4444
#include <ApexProperties.sysprop.h>
4545

46-
#include "init.h"
4746
#include "mount_namespace.h"
4847
#include "property_service.h"
4948
#else
@@ -260,7 +259,7 @@ void Service::Reap(const siginfo_t& siginfo) {
260259

261260
if ((siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) && on_failure_reboot_target_) {
262261
LOG(ERROR) << "Service with 'reboot_on_failure' option failed, shutting down system.";
263-
TriggerShutdown(*on_failure_reboot_target_);
262+
trigger_shutdown(*on_failure_reboot_target_);
264263
}
265264

266265
if (flags_ & SVC_EXEC) UnSetExec();
@@ -340,7 +339,7 @@ void Service::DumpState() const {
340339
Result<void> Service::ExecStart() {
341340
auto reboot_on_failure = make_scope_guard([this] {
342341
if (on_failure_reboot_target_) {
343-
TriggerShutdown(*on_failure_reboot_target_);
342+
trigger_shutdown(*on_failure_reboot_target_);
344343
}
345344
});
346345

@@ -371,7 +370,7 @@ Result<void> Service::ExecStart() {
371370
Result<void> Service::Start() {
372371
auto reboot_on_failure = make_scope_guard([this] {
373372
if (on_failure_reboot_target_) {
374-
TriggerShutdown(*on_failure_reboot_target_);
373+
trigger_shutdown(*on_failure_reboot_target_);
375374
}
376375
});
377376

init/subcontext.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ namespace android {
5151
namespace init {
5252
namespace {
5353

54+
std::string shutdown_command;
55+
5456
class SubcontextProcess {
5557
public:
5658
SubcontextProcess(const BuiltinFunctionMap* function_map, std::string context, int init_fd)
@@ -153,6 +155,11 @@ void SubcontextProcess::MainLoop() {
153155
<< subcontext_command.command_case();
154156
}
155157

158+
if (!shutdown_command.empty()) {
159+
reply.set_trigger_shutdown(shutdown_command);
160+
shutdown_command.clear();
161+
}
162+
156163
if (auto result = SendMessage(init_fd_, reply); !result) {
157164
LOG(FATAL) << "Failed to send message to init: " << result.error();
158165
}
@@ -174,6 +181,8 @@ int SubcontextMain(int argc, char** argv, const BuiltinFunctionMap* function_map
174181
return 0;
175182
};
176183

184+
trigger_shutdown = [](const std::string& command) { shutdown_command = command; };
185+
177186
auto subcontext_process = SubcontextProcess(function_map, context, init_fd);
178187
subcontext_process.MainLoop();
179188
return 0;
@@ -254,6 +263,11 @@ Result<SubcontextReply> Subcontext::TransmitMessage(const SubcontextCommand& sub
254263
Restart();
255264
return Error() << "Unable to parse message from subcontext";
256265
}
266+
267+
if (subcontext_reply.has_trigger_shutdown()) {
268+
trigger_shutdown(subcontext_reply.trigger_shutdown());
269+
}
270+
257271
return subcontext_reply;
258272
}
259273

init/subcontext.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,6 @@ message SubcontextReply {
3838
Failure failure = 2;
3939
ExpandArgsReply expand_args_reply = 3;
4040
}
41+
42+
optional string trigger_shutdown = 4;
4143
}

init/subcontext_test.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <selinux/selinux.h>
2727

2828
#include "builtin_arguments.h"
29+
#include "util.h"
2930

3031
using namespace std::literals;
3132

@@ -142,6 +143,18 @@ TEST(subcontext, ContextString) {
142143
});
143144
}
144145

146+
TEST(subcontext, TriggerShutdown) {
147+
static constexpr const char kTestShutdownCommand[] = "reboot,test-shutdown-command";
148+
static std::string trigger_shutdown_command;
149+
trigger_shutdown = [](const std::string& command) { trigger_shutdown_command = command; };
150+
RunTest([](auto& subcontext, auto& context_string) {
151+
auto result = subcontext.Execute(
152+
std::vector<std::string>{"trigger_shutdown", kTestShutdownCommand});
153+
ASSERT_TRUE(result);
154+
});
155+
EXPECT_EQ(kTestShutdownCommand, trigger_shutdown_command);
156+
}
157+
145158
TEST(subcontext, ExpandArgs) {
146159
RunTest([](auto& subcontext, auto& context_string) {
147160
auto args = std::vector<std::string>{
@@ -207,6 +220,11 @@ BuiltinFunctionMap BuildTestFunctionMap() {
207220
return Error() << args.context;
208221
};
209222

223+
auto do_trigger_shutdown = [](const BuiltinArguments& args) -> Result<void> {
224+
trigger_shutdown(args[1]);
225+
return {};
226+
};
227+
210228
// clang-format off
211229
BuiltinFunctionMap test_function_map = {
212230
{"return_pids_as_error", {0, 0, {true, do_return_pids_as_error}}},
@@ -216,6 +234,7 @@ BuiltinFunctionMap BuildTestFunctionMap() {
216234
{"cause_log_fatal", {0, 0, {true, do_cause_log_fatal}}},
217235
{"generate_sane_error", {0, 0, {true, do_generate_sane_error}}},
218236
{"return_context_as_error", {0, 0, {true, do_return_context_as_error}}},
237+
{"trigger_shutdown", {1, 1, {true, do_trigger_shutdown}}},
219238
};
220239
// clang-format on
221240
return test_function_map;

init/util.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ namespace init {
6161

6262
const std::string kDefaultAndroidDtDir("/proc/device-tree/firmware/android/");
6363

64+
void (*trigger_shutdown)(const std::string& command) = nullptr;
65+
6466
// DecodeUid() - decodes and returns the given string, which can be either the
6567
// numeric or name representation, into the integer uid or gid.
6668
Result<uid_t> DecodeUid(const std::string& name) {

init/util.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ namespace init {
3535

3636
static const char kColdBootDoneProp[] = "ro.cold_boot_done";
3737

38+
extern void (*trigger_shutdown)(const std::string& command);
39+
3840
Result<int> CreateSocket(const std::string& name, int type, bool passcred, mode_t perm, uid_t uid,
3941
gid_t gid, const std::string& socketcon);
4042

0 commit comments

Comments
 (0)