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
diff --git a/init/Android.bp b/init/Android.bp
index ba60085..31e4173 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -227,9 +227,10 @@
 
 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 @@
         "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 97b1c1a..65ba25d 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -68,6 +68,30 @@
     return RunBuiltinFunction(func_, args_, kInitContext);
 }
 
+Result<void> 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 @@
     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 80c1da4..1534bf9 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 <map>
 #include <queue>
@@ -41,6 +40,7 @@
 
     Result<void> InvokeFunc(Subcontext* subcontext) const;
     std::string BuildCommandString() const;
+    Result<void> CheckCommand() const;
 
     int line() const { return line_; }
 
@@ -63,7 +63,7 @@
 
     Result<void> AddCommand(std::vector<std::string>&& args, int line);
     void AddCommand(BuiltinFunction f, std::vector<std::string>&& 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 @@
     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 @@
 
 }  // namespace init
 }  // namespace android
-
-#endif
diff --git a/init/action_manager.cpp b/init/action_manager.cpp
index 541c8f2..ebca762 100644
--- a/init/action_manager.cpp
+++ b/init/action_manager.cpp
@@ -23,6 +23,14 @@
 
 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 5f47a6d..a2b95ac 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 <string>
 #include <vector>
@@ -32,6 +31,7 @@
 
     // Exposed for testing
     ActionManager();
+    size_t CheckAllCommands();
 
     void AddAction(std::unique_ptr<Action> action);
     void QueueEventTrigger(const std::string& trigger);
@@ -55,5 +55,3 @@
 
 }  // namespace init
 }  // namespace android
-
-#endif
diff --git a/init/builtins.cpp b/init/builtins.cpp
index b6e26a1..e75f5cb 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -201,26 +201,26 @@
 static Result<void> 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<void> 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 @@
         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<void> 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 @@
                                             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 @@
             }
         }
     });
-    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 0000000..3bd4774
--- /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 <sys/time.h>
+
+#include <android-base/logging.h>
+#include <android-base/parseint.h>
+#include <android-base/strings.h>
+
+#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<void> 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<void> check_exec(const BuiltinArguments& args) {
+    ReturnIfAnyArgsEmpty();
+
+    auto result = Service::MakeTemporaryOneshotService(args.args);
+    if (!result) {
+        return result.error();
+    }
+
+    return {};
+}
+
+Result<void> check_exec_background(const BuiltinArguments& args) {
+    return check_exec(std::move(args));
+}
+
+Result<void> check_load_system_props(const BuiltinArguments& args) {
+    return Error() << "'load_system_props' is deprecated";
+}
+
+Result<void> 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<void> 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<void> check_restorecon(const BuiltinArguments& args) {
+    ReturnIfAnyArgsEmpty();
+
+    auto restorecon_info = ParseRestorecon(args.args);
+    if (!restorecon_info) {
+        return restorecon_info.error();
+    }
+
+    return {};
+}
+
+Result<void> check_restorecon_recursive(const BuiltinArguments& args) {
+    return check_restorecon(std::move(args));
+}
+
+Result<void> 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<void> check_setrlimit(const BuiltinArguments& args) {
+    ReturnIfAnyArgsEmpty();
+
+    auto rlimit = ParseRlimit(args.args);
+    if (!rlimit) return rlimit.error();
+    return {};
+}
+
+Result<void> 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<void> 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<void> 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 0000000..c974e88
--- /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<void> check_chown(const BuiltinArguments& args);
+Result<void> check_exec(const BuiltinArguments& args);
+Result<void> check_exec_background(const BuiltinArguments& args);
+Result<void> check_load_system_props(const BuiltinArguments& args);
+Result<void> check_loglevel(const BuiltinArguments& args);
+Result<void> check_mkdir(const BuiltinArguments& args);
+Result<void> check_restorecon(const BuiltinArguments& args);
+Result<void> check_restorecon_recursive(const BuiltinArguments& args);
+Result<void> check_setprop(const BuiltinArguments& args);
+Result<void> check_setrlimit(const BuiltinArguments& args);
+Result<void> check_sysclktz(const BuiltinArguments& args);
+Result<void> check_wait(const BuiltinArguments& args);
+Result<void> 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 0000000..6afcb17
--- /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 dfde51b..a5a5b1b 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 @@
 namespace android {
 namespace init {
 
-static Result<void> do_stub(const BuiltinArguments& args) {
+static Result<void> check_stub(const BuiltinArguments& args) {
     return {};
 }
 
@@ -238,9 +239,10 @@
         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 3819042..17622a3 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -174,13 +174,8 @@
         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<std::size_t>(-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 380c93b..e60c20d 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -635,7 +635,8 @@
     }
 }
 
-std::unique_ptr<Service> Service::MakeTemporaryOneshotService(const std::vector<std::string>& args) {
+Result<std::unique_ptr<Service>> Service::MakeTemporaryOneshotService(
+        const std::vector<std::string>& 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 @@
         }
     }
     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<std::string> str_args(args.begin() + command_arg, args.end());
 
@@ -671,8 +670,7 @@
     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_t> gid = 0;
@@ -680,16 +678,14 @@
     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 cdf31bb..6f79faa 100644
--- a/init/service.h
+++ b/init/service.h
@@ -71,7 +71,8 @@
             const std::vector<gid_t>& supp_gids, int namespace_flags, const std::string& seclabel,
             Subcontext* subcontext_for_restart_commands, const std::vector<std::string>& args);
 
-    static std::unique_ptr<Service> MakeTemporaryOneshotService(const std::vector<std::string>& args);
+    static Result<std::unique_ptr<Service>> MakeTemporaryOneshotService(
+            const std::vector<std::string>& args);
 
     bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; }
     Result<void> ExecStart();
diff --git a/init/service_test.cpp b/init/service_test.cpp
index 6a34acc..c9cc7bd 100644
--- a/init/service_test.cpp
+++ b/init/service_test.cpp
@@ -75,15 +75,15 @@
 TEST(service, make_temporary_oneshot_service_invalid_syntax) {
     std::vector<std::string> 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 @@
     }
     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 @@
     }
     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 169f086..0532375 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -42,6 +42,7 @@
 
 #if defined(__ANDROID__)
 #include <android/api-level.h>
+#include <sys/system_properties.h>
 
 #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 @@
     return true;
 }
 
+Result<void> 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<std::size_t>(-1)) {
+        return Error() << "Value is not a UTF8 encoded string";
+    }
+
+    return {};
+}
+
+Result<std::pair<int, std::vector<std::string>>> ParseRestorecon(
+        const std::vector<std::string>& 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<std::string> 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 98de1f2..4cccefe 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 <sys/stat.h>
 #include <sys/types.h>
 
 #include <chrono>
 #include <functional>
-#include <ostream>
 #include <string>
 
 #include <android-base/chrono_utils.h>
-#include <selinux/label.h>
 
 #include "result.h"
 
 using android::base::boot_clock;
-using namespace std::chrono_literals;
 
 namespace android {
 namespace init {
@@ -62,11 +58,13 @@
 bool is_android_dt_value_expected(const std::string& sub_path, const std::string& expected_content);
 
 bool IsLegalPropertyName(const std::string& name);
+Result<void> IsLegalPropertyValue(const std::string& name, const std::string& value);
+
+Result<std::pair<int, std::vector<std::string>>> ParseRestorecon(
+        const std::vector<std::string>& args);
 
 void SetStdioToDevNull(char** argv);
 void InitKernelLogging(char** argv);
 bool IsRecoveryMode();
 }  // namespace init
 }  // namespace android
-
-#endif