From 5e405cacb1f3ef2265fcc71dbbf6a39c1e7dc1c1 Mon Sep 17 00:00:00 2001
From: Tom Cherry <tomcherry@google.com>
Date: Mon, 11 Sep 2017 16:08:54 -0700
Subject: [PATCH] init: fix variable scope issue with ExpandArgs()

ExpandArgs() was factored out of Service::Start() to clean up init,
however this introduced a bug: the scope of expanded_args ends when
ExpandArgs() returns, yet pointers to the c strings contained within
those std::strings are returned from the function.  These pointers are
invalid and have been seen to cause failures on real devices.

This change moves the execv() into ExpandArgs() and renames it
ExpandArgsAndExecv() to keep the clean separation of Service::Start()
but fix the variable scope issue.

Bug: 65303004
Test: boot fugu
Change-Id: I612128631f5b58d040bffcbc2220593ad16cd450
---
 init/service.cpp | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/init/service.cpp b/init/service.cpp
index 6f27a4bb6..86b910ae6 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -135,17 +135,21 @@ static void SetUpPidNamespace(const std::string& service_name) {
     }
 }
 
-static void ExpandArgs(const std::vector<std::string>& args, std::vector<char*>* strs) {
+static bool ExpandArgsAndExecv(const std::vector<std::string>& args) {
     std::vector<std::string> expanded_args;
+    std::vector<char*> c_strings;
+
     expanded_args.resize(args.size());
-    strs->push_back(const_cast<char*>(args[0].c_str()));
+    c_strings.push_back(const_cast<char*>(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] << "'";
         }
-        strs->push_back(const_cast<char*>(expanded_args[i].c_str()));
+        c_strings.push_back(expanded_args[i].data());
     }
-    strs->push_back(nullptr);
+    c_strings.push_back(nullptr);
+
+    return execv(c_strings[0], c_strings.data()) == 0;
 }
 
 unsigned long Service::next_start_order_ = 1;
@@ -785,10 +789,8 @@ Result<Success> Service::Start() {
         // priority. Aborts on failure.
         SetProcessAttributes();
 
-        std::vector<char*> strs;
-        ExpandArgs(args_, &strs);
-        if (execv(strs[0], (char**)&strs[0]) < 0) {
-            PLOG(ERROR) << "cannot execve('" << strs[0] << "')";
+        if (!ExpandArgsAndExecv(args_)) {
+            PLOG(ERROR) << "cannot execve('" << args_[0] << "')";
         }
 
         _exit(127);