minijail_wait returns 126 or 127 if execve fails in child process

This is more consistent than returning 255.

See "Exit Status for Commands":
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02

Bug: https://crbug.com/1012561
Change-Id: I1ab5c08628719f58b7395b0e55bc4a0b0032bd56
Tests: Unit tests pass

3 files changed, 95 insertions, 7 deletions

diff --git a/libminijail.c b/libminijail.c
index cd6ffe76..c9948bc5 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -3102,10 +3102,10 @@ static int minijail_run_internal(struct minijail *j,
 	 *   -> init()-ing process
 	 *      -> execve()-ing process
 	 */
-	ret = execve(config->filename, config->argv, child_env);
-	if (ret == -1) {
-		pwarn("execve(%s) failed", config->filename);
-	}
+	execve(config->filename, config->argv, child_env);
+
+	ret = (errno == ENOENT ? MINIJAIL_ERR_NO_COMMAND : MINIJAIL_ERR_NO_ACCESS);
+	pwarn("execve(%s) failed", config->filename);
 	_exit(ret);
 }
 
@@ -3121,6 +3121,9 @@ int API minijail_kill(struct minijail *j)
 
 int API minijail_wait(struct minijail *j)
 {
+	if (j->initpid <= 0)
+		return -ECHILD;
+
 	int st;
 	if (waitpid(j->initpid, &st, 0) < 0)
 		return -errno;
@@ -3141,7 +3144,7 @@ int API minijail_wait(struct minijail *j)
 			if (signum == SIGSYS) {
 				error_status = MINIJAIL_ERR_JAIL;
 			} else {
-				error_status = 128 + signum;
+				error_status = MINIJAIL_ERR_SIG_BASE + signum;
 			}
 		}
 		return error_status;
diff --git a/libminijail.h b/libminijail.h
index 5124741d..67c515c7 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -23,9 +23,22 @@
 extern "C" {
 #endif
 
+/* Possible exit status codes returned by minijail_wait(). */
 enum {
+	/* Command can be found but cannot be run */
+	MINIJAIL_ERR_NO_ACCESS = 126,
+
+	/* Command cannot be found */
+	MINIJAIL_ERR_NO_COMMAND = 127,
+
+	/* (MINIJAIL_ERR_SIG_BASE + n) if process killed by signal n != SIGSYS */
+	MINIJAIL_ERR_SIG_BASE = 128,
+
 	MINIJAIL_ERR_PRELOAD = 252,
+
+	/* Process killed by SIGSYS */
 	MINIJAIL_ERR_JAIL = 253,
+
 	MINIJAIL_ERR_INIT = 254,
 };
 
@@ -397,8 +410,18 @@ pid_t minijail_fork(struct minijail *j);
 int minijail_kill(struct minijail *j);
 
 /*
- * Wait for the _first_ process spawned in the specified minijail to exit, and
- * return its exit status.
+ * Wait for the first process spawned in the specified minijail to exit, and
+ * return its exit status. A process can only be awaited once.
+ *
+ * Return:
+ *   A negative error code if the process cannot be awaited for (eg -ECHILD if
+ *   no process has been started or if the process has already been awaited
+ *   for).
+ *   MINIJAIL_ERR_NO_COMMAND if command cannot be found.
+ *   MINIJAIL_ERR_NO_ACCESS if command cannot be run.
+ *   MINIJAIL_ERR_JAIL if process was killed by SIGSYS.
+ *   (MINIJAIL_ERR_SIG_BASE  + n) if process was killed by signal n != SIGSYS.
+ *   (n & 0xFF) if process finished by returning code n.
  */
 int minijail_wait(struct minijail *j);
 
diff --git a/libminijail_unittest.cc b/libminijail_unittest.cc
index 9a046244..d090d500 100644
--- a/libminijail_unittest.cc
+++ b/libminijail_unittest.cc
@@ -217,6 +217,68 @@ TEST_F(MarshalTest, 0xff) {
   EXPECT_EQ(-EINVAL, minijail_unmarshal(j_, buf_, sizeof(buf_)));
 }
 
+TEST(WaitTest, return_zero) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "exit 0", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), 0);
+}
+
+TEST(WaitTest, return_max) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "exit 255", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), 255);
+}
+
+TEST(WaitTest, return_modulo) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "exit 256", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), 0);
+}
+
+TEST(WaitTest, killed_by_sigkill) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "kill -KILL $$; sleep 1000", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), MINIJAIL_ERR_SIG_BASE  + SIGKILL);
+}
+
+TEST(WaitTest, killed_by_sigsys) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "kill -SYS $$; sleep 1000", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), MINIJAIL_ERR_JAIL);
+}
+
+TEST(WaitTest, command_not_found) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"whatever", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), "command that cannot be found", argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), MINIJAIL_ERR_NO_COMMAND);
+}
+
+TEST(WaitTest, command_not_run) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"whatever", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), "/dev/null", argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), MINIJAIL_ERR_NO_ACCESS);
+}
+
+TEST(WaitTest, no_process) {
+  const ScopedMinijail j(minijail_new());
+  EXPECT_EQ(minijail_wait(j.get()), -ECHILD);
+}
+
+TEST(WaitTest, can_wait_only_once) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "exit 0", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), 0);
+  EXPECT_EQ(minijail_wait(j.get()), -ECHILD);
+}
+
 TEST(Test, minijail_run_pid_pipes) {
   // TODO(crbug.com/895875): The preload library interferes with ASan since they
   // both need to use LD_PRELOAD.