commit 2e8a9845e3190815d5fbdb1756e40a5554977733
author Martijn Coenen <maco@google.com> Tue Jul 27 13:40:23 2021 +0200
committer Martijn Coenen <maco@google.com> Wed Jul 28 11:06:06 2021 +0200
tree 8b659ea682e264d9b6462c49b19157bac987eeb6
parent a68ba133238ba39a9209838ce867a55146c9084a

Don't mark odsign as oneshot.

If odsign is marked as oneshot, and it crashes (eg due to a coding
error), the device will not boot completely, because init keeps waiting
for the odsign.key.done / odsign.verification.done properties. So
instead, we don't mark it as oneshot, but stop the service manually in
the exit paths of the code. This ensures that if a bad OTA / module
update causes odsign to crash, we will automatically start it again; if
it crashes repeatedly, apexd will detect this, and roll back any module
update.

In the good path, there's no difference - odsign will run just once and
be stopped.

Bug: 194334176
Test: manually make odsign crash; inspect output
Change-Id: I7015f291888d6b8066e4c526a7e8cf3c9c7ea618

ondevice-signing/odsign.rc

diff --git a/ondevice-signing/odsign.rc b/ondevice-signing/odsign.rc
index 044bae7..de09fc0 100644
--- a/ondevice-signing/odsign.rc
+++ b/ondevice-signing/odsign.rc
@@ -2,5 +2,8 @@
     class core
     user root
     group system
-    oneshot
     disabled # does not start with the core class
+
+# Note that odsign is not oneshot, but stopped manually when it exits. This
+# ensures that if odsign crashes during a module update, apexd will detect
+# those crashes and roll back the update.

ondevice-signing/odsign_main.cpp

diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp
index ff7a105..699049e 100644
--- a/ondevice-signing/odsign_main.cpp
+++ b/ondevice-signing/odsign_main.cpp
@@ -75,6 +75,8 @@
 static const char* kOdsignVerificationStatusValid = "1";
 static const char* kOdsignVerificationStatusError = "0";
 
+static const char* kStopServiceProp = "ctl.stop";
+
 static void writeBytesToFile(const std::vector<uint8_t>& bytes, const std::string& path) {
     std::string str(bytes.begin(), bytes.end());
     android::base::WriteStringToFile(str, path);
@@ -552,8 +554,10 @@
         // Tell init we don't need to use our key anymore
         SetProperty(kOdsignKeyDoneProp, "1");
         // Tell init we're done with verification, and that it was an error
-        SetProperty(kOdsignVerificationDoneProp, "1");
         SetProperty(kOdsignVerificationStatusProp, kOdsignVerificationStatusError);
+        SetProperty(kOdsignVerificationDoneProp, "1");
+        // Tell init it shouldn't try to restart us - see odsign.rc
+        SetProperty(kStopServiceProp, "odsign");
     };
     auto scope_guard = android::base::make_scope_guard(errorScopeGuard);
 
@@ -668,8 +672,10 @@
     // At this point, we're done with the key for sure
     SetProperty(kOdsignKeyDoneProp, "1");
     // And we did a successful verification
-    SetProperty(kOdsignVerificationDoneProp, "1");
     SetProperty(kOdsignVerificationStatusProp, kOdsignVerificationStatusValid);
+    SetProperty(kOdsignVerificationDoneProp, "1");
 
+    // Tell init it shouldn't try to restart us - see odsign.rc
+    SetProperty(kStopServiceProp, "odsign");
     return 0;
 }