Allow passing environment also for LD_PRELOAD am: b35f2c18a0 am: 0a4d9de5f1

Change-Id: Idc4e831bdb1c4893322b78257672653fd0933b49

6 files changed, 320 insertions, 144 deletions

diff --git a/libminijail.c b/libminijail.c
index 784509c..7a82ccc 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -2408,7 +2408,7 @@ error:
 }
 
 static int setup_preload(const struct minijail *j attribute_unused,
-			 const char *oldenv attribute_unused)
+			 char ***child_env attribute_unused)
 {
 #if defined(__ANDROID__)
 	/* Don't use LDPRELOAD on Android. */
@@ -2417,6 +2417,7 @@ static int setup_preload(const struct minijail *j attribute_unused,
 	const char *preload_path = j->preload_path ?: PRELOADPATH;
 	char *newenv = NULL;
 	int ret = 0;
+	const char *oldenv = getenv(kLdPreloadEnvVar);
 
 	if (!oldenv)
 		oldenv = "";
@@ -2427,19 +2428,13 @@ static int setup_preload(const struct minijail *j attribute_unused,
 		return -1;
 	}
 
-	/*
-	 * Avoid using putenv(3), since that requires us to hold onto a
-	 * reference to that string until the environment is no longer used to
-	 * prevent a memory leak.
-	 * See https://crbug.com/930189 for more details.
-	 */
-	ret = setenv(kLdPreloadEnvVar, newenv, 1);
+	ret = minijail_setenv(child_env, kLdPreloadEnvVar, newenv, 1);
 	free(newenv);
 	return ret;
 #endif
 }
 
-static int setup_pipe(int fds[2])
+static int setup_pipe(char ***child_env, int fds[2])
 {
 	int r = pipe(fds);
 	char fd_buf[11];
@@ -2448,8 +2443,7 @@ static int setup_pipe(int fds[2])
 	r = snprintf(fd_buf, sizeof(fd_buf), "%d", fds[0]);
 	if (r <= 0)
 		return -EINVAL;
-	setenv(kFdEnvVar, fd_buf, 1);
-	return 0;
+	return minijail_setenv(child_env, kFdEnvVar, fd_buf, 1);
 }
 
 static int close_open_fds(int *inheritable_fds, size_t size)
@@ -2504,20 +2498,17 @@ static int redirect_fds(struct minijail *j)
  * Structure holding resources and state created when running a minijail.
  */
 struct minijail_run_state {
-	char *oldenv;
 	pid_t child_pid;
 	int pipe_fds[2];
 	int stdin_fds[2];
 	int stdout_fds[2];
 	int stderr_fds[2];
 	int child_sync_pipe_fds[2];
+	char **child_env;
 };
 
 static void minijail_free_run_state(struct minijail_run_state *state)
 {
-	free(state->oldenv);
-	state->oldenv = NULL;
-
 	state->child_pid = -1;
 
 	int *fd_pairs[] = {state->pipe_fds, state->stdin_fds, state->stdout_fds,
@@ -2526,6 +2517,9 @@ static void minijail_free_run_state(struct minijail_run_state *state)
 		close_and_reset(&fd_pairs[i][0]);
 		close_and_reset(&fd_pairs[i][1]);
 	}
+
+	minijail_free_env(state->child_env);
+	state->child_env = NULL;
 }
 
 /* Set up stdin/stdout/stderr file descriptors in the child. */
@@ -2580,7 +2574,6 @@ static void setup_child_std_fds(struct minijail *j,
  * filename - The program to exec in the child. Required if |exec_in_child| = 1.
  * argv - Arguments for the child program. Required if |exec_in_child| = 1.
  * envp - Environment for the child program. Available if |exec_in_child| = 1.
-       Currently only honored if |use_preload| = 0 and non-NULL.
  * use_preload - If true use LD_PRELOAD.
  * exec_in_child - If true, run |filename|. Otherwise, the child will return to
  *     the caller.
@@ -2664,6 +2657,25 @@ int API minijail_run_pid_pipes(struct minijail *j, const char *filename,
 	return minijail_run_config_internal(j, &config);
 }
 
+int API minijail_run_env_pid_pipes(struct minijail *j, const char *filename,
+				   char *const argv[], char *const envp[],
+				   pid_t *pchild_pid, int *pstdin_fd,
+				   int *pstdout_fd, int *pstderr_fd)
+{
+	struct minijail_run_config config = {
+	    .filename = filename,
+	    .argv = argv,
+	    .envp = envp,
+	    .use_preload = true,
+	    .exec_in_child = true,
+	    .pstdin_fd = pstdin_fd,
+	    .pstdout_fd = pstdout_fd,
+	    .pstderr_fd = pstderr_fd,
+	    .pchild_pid = pchild_pid,
+	};
+	return minijail_run_config_internal(j, &config);
+}
+
 int API minijail_run_no_preload(struct minijail *j, const char *filename,
 				char *const argv[])
 {
@@ -2746,17 +2758,17 @@ static int minijail_run_internal(struct minijail *j,
 			die("Minijail hooks are not supported with LD_PRELOAD");
 		if (!config->exec_in_child)
 			die("minijail_fork is not supported with LD_PRELOAD");
-		if (config->envp != NULL)
-			die("cannot pass a new environment with LD_PRELOAD");
-
-		char *oldenv = getenv(kLdPreloadEnvVar);
-		if (oldenv) {
-			state_out->oldenv = strdup(oldenv);
-			if (!state_out->oldenv)
-				return -ENOMEM;
-		}
 
-		if (setup_preload(j, oldenv))
+		/*
+		 * Before we fork(2) and execve(2) the child process, we need
+		 * to open a pipe(2) to send the minijail configuration over.
+		 */
+		state_out->child_env =
+		    minijail_copy_env(config->envp ? config->envp : environ);
+		if (!state_out->child_env)
+			return ENOMEM;
+		if (setup_preload(j, &state_out->child_env) ||
+		    setup_pipe(&state_out->child_env, state_out->pipe_fds))
 			return -EFAULT;
 	}
 
@@ -2768,15 +2780,6 @@ static int minijail_run_internal(struct minijail *j,
 		}
 	}
 
-	if (use_preload) {
-		/*
-		 * Before we fork(2) and execve(2) the child process, we need
-		 * to open a pipe(2) to send the minijail configuration over.
-		 */
-		if (setup_pipe(state_out->pipe_fds))
-			return -EFAULT;
-	}
-
 	/* Create pipes for stdin/stdout/stderr as requested by caller. */
 	struct {
 		bool requested;
@@ -2869,16 +2872,6 @@ static int minijail_run_internal(struct minijail *j,
 
 	state_out->child_pid = child_pid;
 	if (child_pid) {
-		if (use_preload) {
-			/* Restore parent's LD_PRELOAD. */
-			if (state_out->oldenv) {
-				setenv(kLdPreloadEnvVar, state_out->oldenv, 1);
-			} else {
-				unsetenv(kLdPreloadEnvVar);
-			}
-			unsetenv(kFdEnvVar);
-		}
-
 		j->initpid = child_pid;
 
 		if (j->flags.forward_signals) {
@@ -3090,25 +3083,19 @@ static int minijail_run_internal(struct minijail *j,
 
 	/*
 	 * We're going to execve(), so make sure any remaining resources are
-	 * freed. The exception is the read side of the LD_PRELOAD pipe, which
-	 * we need to hand down into the target.
+	 * freed. Exceptions are:
+	 *  1. The child environment. No need to worry about freeing it since
+	 *     execve reinitializes the heap anyways.
+	 *  2. The read side of the LD_PRELOAD pipe, which we need to hand down
+	 *     into the target in which the preloaded code will read from it and
+	 *     then close it.
 	 */
 	state_out->pipe_fds[0] = -1;
+	char *const *child_env = state_out->child_env;
+	state_out->child_env = NULL;
 	minijail_free_run_state(state_out);
 
 	/*
-	 * If not using LD_PRELOAD, support passing a new environment instead of
-	 * inheriting the parent's.
-	 * When not using LD_PRELOAD there is no need to modify the environment
-	 * to add Minijail-related variables, so passing a new environment is
-	 * fine.
-	 */
-	char *const *child_env = environ;
-	if (!use_preload && config->envp != NULL) {
-		child_env = config->envp;
-	}
-
-	/*
 	 * If we aren't pid-namespaced, or the jailed program asked to be init:
 	 *   calling process
 	 *   -> execve()-ing process
@@ -3117,6 +3104,8 @@ static int minijail_run_internal(struct minijail *j,
 	 *   -> init()-ing process
 	 *      -> execve()-ing process
 	 */
+	if (!child_env)
+		child_env = config->envp ? config->envp : environ;
 	execve(config->filename, config->argv, child_env);
 
 	ret = (errno == ENOENT ? MINIJAIL_ERR_NO_COMMAND : MINIJAIL_ERR_NO_ACCESS);
@@ -3129,13 +3118,13 @@ minijail_run_config_internal(struct minijail *j,
 			     const struct minijail_run_config *config)
 {
 	struct minijail_run_state state = {
-	    .oldenv = NULL,
 	    .child_pid = -1,
 	    .pipe_fds = {-1, -1},
 	    .stdin_fds = {-1, -1},
 	    .stdout_fds = {-1, -1},
 	    .stderr_fds = {-1, -1},
 	    .child_sync_pipe_fds = {-1, -1},
+	    .child_env = NULL,
 	};
 	int ret = minijail_run_internal(j, config, &state);
 
diff --git a/libminijail.h b/libminijail.h
index 286369e..9eefe0a 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -365,6 +365,22 @@ int minijail_run_pid_pipes(struct minijail *j, const char *filename,
 
 /*
  * Run the specified command in the given minijail, execve(2)-style.
+ * Pass |envp| as the full environment for the child.
+ * Update |*pchild_pid| with the pid of the child.
+ * Update |*pstdin_fd| with a fd that allows writing to the child's
+ * standard input.
+ * Update |*pstdout_fd| with a fd that allows reading from the child's
+ * standard output.
+ * Update |*pstderr_fd| with a fd that allows reading from the child's
+ * standard error.
+ */
+int minijail_run_env_pid_pipes(struct minijail *j, const char *filename,
+			       char *const argv[], char *const envp[],
+			       pid_t *pchild_pid, int *pstdin_fd,
+			       int *pstdout_fd, int *pstderr_fd);
+
+/*
+ * Run the specified command in the given minijail, execve(2)-style.
  * Update |*pchild_pid| with the pid of the child.
  * Update |*pstdin_fd| with a fd that allows writing to the child's
  * standard input.
diff --git a/libminijail_unittest.cc b/libminijail_unittest.cc
index d8ffc38..18d016d 100644
--- a/libminijail_unittest.cc
+++ b/libminijail_unittest.cc
@@ -381,153 +381,126 @@ TEST(Test, close_original_pipes_after_dup2) {
   EXPECT_EQ(minijail_wait(j.get()), 42);
 }
 
-TEST(Test, minijail_run_pid_pipes) {
+TEST(Test, minijail_run_env_pid_pipes) {
   // TODO(crbug.com/895875): The preload library interferes with ASan since they
   // both need to use LD_PRELOAD.
   if (running_with_asan()) {
     SUCCEED();
     return;
   }
-  constexpr char teststr[] = "test\n";
 
   ScopedMinijail j(minijail_new());
   minijail_set_preload_path(j.get(), kPreloadPath);
 
-  char* argv[4];
+  char *argv[4];
   argv[0] = const_cast<char*>(kCatPath);
-  argv[1] = nullptr;
+  argv[1] = NULL;
+
   pid_t pid;
   int child_stdin, child_stdout;
-  int mj_run_ret = minijail_run_pid_pipes(j.get(), argv[0], argv, &pid,
-                                          &child_stdin, &child_stdout, nullptr);
+  int mj_run_ret = minijail_run_pid_pipes(
+      j.get(), argv[0], argv, &pid, &child_stdin, &child_stdout, NULL);
   EXPECT_EQ(mj_run_ret, 0);
 
+  char teststr[] = "test\n";
   const size_t teststr_len = strlen(teststr);
   ssize_t write_ret = write(child_stdin, teststr, teststr_len);
   EXPECT_EQ(write_ret, static_cast<ssize_t>(teststr_len));
 
-  char buf[kBufferSize];
-  ssize_t read_ret = read(child_stdout, buf, 8);
+  char buf[kBufferSize] = {};
+  ssize_t read_ret = read(child_stdout, buf, sizeof(buf) - 1);
   EXPECT_EQ(read_ret, static_cast<ssize_t>(teststr_len));
-  buf[teststr_len] = 0;
-  EXPECT_EQ(strcmp(buf, teststr), 0);
+  EXPECT_STREQ(buf, teststr);
 
   int status;
   EXPECT_EQ(kill(pid, SIGTERM), 0);
-  waitpid(pid, &status, 0);
+  EXPECT_EQ(waitpid(pid, &status, 0), pid);
   ASSERT_TRUE(WIFSIGNALED(status));
   EXPECT_EQ(WTERMSIG(status), SIGTERM);
 
   argv[0] = const_cast<char*>(kShellPath);
   argv[1] = "-c";
-  argv[2] = "echo test >&2";
+  argv[2] = "echo \"${TEST_PARENT+set}|${TEST_VAR}\" >&2";
   argv[3] = nullptr;
+
+  char *envp[2];
+  envp[0] = "TEST_VAR=test";
+  envp[1] = NULL;
+
+  // Set a canary env var in the parent that should not be present in the child.
+  ASSERT_EQ(setenv("TEST_PARENT", "test", 1 /*overwrite*/), 0);
+
   int child_stderr;
-  mj_run_ret = minijail_run_pid_pipes(j.get(), argv[0], argv, &pid,
-                                      &child_stdin, &child_stdout,
-                                      &child_stderr);
+  mj_run_ret =
+      minijail_run_env_pid_pipes(j.get(), argv[0], argv, envp, &pid,
+                                 &child_stdin, &child_stdout, &child_stderr);
   EXPECT_EQ(mj_run_ret, 0);
 
-  read_ret = read(child_stderr, buf, sizeof(buf));
-  EXPECT_GE(read_ret, static_cast<ssize_t>(teststr_len));
+  memset(buf, 0, sizeof(buf));
+  read_ret = read(child_stderr, buf, sizeof(buf) - 1);
+  EXPECT_GE(read_ret, 0);
+  EXPECT_STREQ(buf, "|test\n");
 
-  waitpid(pid, &status, 0);
+  EXPECT_EQ(waitpid(pid, &status, 0), pid);
   ASSERT_TRUE(WIFEXITED(status));
   EXPECT_EQ(WEXITSTATUS(status), 0);
 }
 
-TEST(Test, minijail_run_pid_pipes_no_preload) {
-  pid_t pid;
-  int child_stdin, child_stdout, child_stderr;
-  int mj_run_ret;
-  ssize_t write_ret, read_ret;
-  char buf[kBufferSize];
-  int status;
-  char teststr[] = "test\n";
-  size_t teststr_len = strlen(teststr);
-  char *argv[4];
-
-  struct minijail *j = minijail_new();
+TEST(Test, minijail_run_env_pid_pipes_no_preload) {
+  ScopedMinijail j(minijail_new());
 
+  char *argv[4];
   argv[0] = const_cast<char*>(kCatPath);
   argv[1] = NULL;
-  mj_run_ret = minijail_run_pid_pipes_no_preload(j, argv[0], argv,
-                                                 &pid,
-                                                 &child_stdin, &child_stdout,
-                                                 NULL);
+
+  pid_t pid;
+  int child_stdin, child_stdout;
+  int mj_run_ret = minijail_run_pid_pipes(
+      j.get(), argv[0], argv, &pid, &child_stdin, &child_stdout, NULL);
   EXPECT_EQ(mj_run_ret, 0);
 
-  write_ret = write(child_stdin, teststr, teststr_len);
-  EXPECT_EQ(write_ret, (int)teststr_len);
+  char teststr[] = "test\n";
+  const size_t teststr_len = strlen(teststr);
+  ssize_t write_ret = write(child_stdin, teststr, teststr_len);
+  EXPECT_EQ(write_ret, static_cast<ssize_t>(teststr_len));
 
-  read_ret = read(child_stdout, buf, 8);
-  EXPECT_EQ(read_ret, (int)teststr_len);
-  buf[teststr_len] = 0;
-  EXPECT_EQ(strcmp(buf, teststr), 0);
+  char buf[kBufferSize] = {};
+  ssize_t read_ret = read(child_stdout, buf, 8);
+  EXPECT_EQ(read_ret, static_cast<ssize_t>(teststr_len));
+  EXPECT_STREQ(buf, teststr);
 
+  int status;
   EXPECT_EQ(kill(pid, SIGTERM), 0);
-  waitpid(pid, &status, 0);
+  EXPECT_EQ(waitpid(pid, &status, 0), pid);
   ASSERT_TRUE(WIFSIGNALED(status));
   EXPECT_EQ(WTERMSIG(status), SIGTERM);
 
   argv[0] = const_cast<char*>(kShellPath);
   argv[1] = "-c";
-  argv[2] = "echo test >&2";
-  argv[3] = NULL;
-  mj_run_ret = minijail_run_pid_pipes_no_preload(j, argv[0], argv, &pid,
-                                                 &child_stdin, &child_stdout,
-                                                 &child_stderr);
-  EXPECT_EQ(mj_run_ret, 0);
-
-  read_ret = read(child_stderr, buf, sizeof(buf));
-  EXPECT_GE(read_ret, (int)teststr_len);
-
-  waitpid(pid, &status, 0);
-  ASSERT_TRUE(WIFEXITED(status));
-  EXPECT_EQ(WEXITSTATUS(status), 0);
-
-  minijail_destroy(j);
-}
+  argv[2] = "echo \"${TEST_PARENT+set}|${TEST_VAR}\" >&2";
+  argv[3] = nullptr;
 
-TEST(Test, minijail_run_env_pid_pipes_no_preload) {
-  pid_t pid;
-  int child_stdin, child_stdout, child_stderr;
-  int mj_run_ret;
-  ssize_t read_ret;
-  char buf[kBufferSize];
-  int status;
-  char test_envvar[] = "TEST_VAR=test";
-  size_t testvar_len = strlen("test");
-  char *argv[4];
   char *envp[2];
-
-  struct minijail *j = minijail_new();
-
-  argv[0] = const_cast<char*>(kShellPath);
-  argv[1] = "-c";
-  argv[2] = "echo \"${TEST_PARENT+set}|${TEST_VAR}\"";
-  argv[3] = NULL;
-
-  envp[0] = test_envvar;
+  envp[0] = "TEST_VAR=test";
   envp[1] = NULL;
 
   // Set a canary env var in the parent that should not be present in the child.
   ASSERT_EQ(setenv("TEST_PARENT", "test", 1 /*overwrite*/), 0);
 
-  mj_run_ret = minijail_run_env_pid_pipes_no_preload(
-      j, argv[0], argv, envp, &pid, &child_stdin, &child_stdout, &child_stderr);
+  int child_stderr;
+  mj_run_ret =
+      minijail_run_env_pid_pipes(j.get(), argv[0], argv, envp, &pid,
+                                 &child_stdin, &child_stdout, &child_stderr);
   EXPECT_EQ(mj_run_ret, 0);
 
-  read_ret = read(child_stdout, buf, sizeof(buf));
-  EXPECT_EQ(read_ret, (int)testvar_len + 2);
-
-  EXPECT_EQ("|test\n", std::string(buf, 0, testvar_len + 2));
+  memset(buf, 0, sizeof(buf));
+  read_ret = read(child_stderr, buf, sizeof(buf));
+  EXPECT_GE(read_ret, 0);
+  EXPECT_STREQ(buf, "|test\n");
 
   EXPECT_EQ(waitpid(pid, &status, 0), pid);
   ASSERT_TRUE(WIFEXITED(status));
   EXPECT_EQ(WEXITSTATUS(status), 0);
-
-  minijail_destroy(j);
 }
 
 TEST(Test, test_minijail_no_fd_leaks) {
diff --git a/util.c b/util.c
index 27284c3..48c3059 100644
--- a/util.c
+++ b/util.c
@@ -3,6 +3,8 @@
  * found in the LICENSE file.
  */
 
+#define _GNU_SOURCE
+
 #include "util.h"
 
 #include <ctype.h>
@@ -462,3 +464,82 @@ void init_logging(enum logging_system_t logger, int fd, int min_priority)
 	logging_config.fd = fd;
 	logging_config.min_priority = min_priority;
 }
+
+void minijail_free_env(char **env)
+{
+	if (!env)
+		return;
+
+	for (char **entry = env; *entry; ++entry) {
+		free(*entry);
+	}
+
+	free(env);
+}
+
+char **minijail_copy_env(char *const *env)
+{
+	if (!env)
+		return calloc(1, sizeof(char *));
+
+	int len = 0;
+	while (env[len])
+		++len;
+
+	char **copy = calloc(len + 1, sizeof(char *));
+	if (!copy)
+		return NULL;
+
+	for (char **entry = copy; *env; ++env, ++entry) {
+		*entry = strdup(*env);
+		if (!*entry) {
+			minijail_free_env(copy);
+			return NULL;
+		}
+	}
+
+	return copy;
+}
+
+int minijail_setenv(char ***env, const char *name, const char *value,
+		    int overwrite)
+{
+	if (!env || !*env || !name || !*name || !value)
+		return EINVAL;
+
+	size_t name_len = strlen(name);
+
+	char **dest = NULL;
+	size_t env_len = 0;
+	for (char **entry = *env; *entry; ++entry, ++env_len) {
+		if (!dest && strncmp(name, *entry, name_len) == 0 &&
+		    (*entry)[name_len] == '=') {
+			if (!overwrite)
+				return 0;
+
+			dest = entry;
+		}
+	}
+
+	char *new_entry = NULL;
+	if (asprintf(&new_entry, "%s=%s", name, value) == -1)
+		return ENOMEM;
+
+	if (dest) {
+		free(*dest);
+		*dest = new_entry;
+		return 0;
+	}
+
+	env_len++;
+	char **new_env = realloc(*env, (env_len + 1) * sizeof(char *));
+	if (!new_env) {
+		free(new_entry);
+		return ENOMEM;
+	}
+
+	new_env[env_len - 1] = new_entry;
+	new_env[env_len] = NULL;
+	*env = new_env;
+	return 0;
+}
diff --git a/util.h b/util.h
index a71fd29..370ee18 100644
--- a/util.h
+++ b/util.h
@@ -195,6 +195,46 @@ char *consumestr(char **buf, size_t *buflength);
  */
 void init_logging(enum logging_system_t logger, int fd, int min_priority);
 
+/*
+ * minjail_free_env: Frees an environment array plus the environment strings it
+ * points to. The environment and its constituent strings must have been
+ * allocated (as opposed to pointing to static data), e.g. by using
+ * minijail_copy_env() and minijail_setenv().
+ *
+ * @env The environment to free.
+ */
+void minijail_free_env(char **env);
+
+/*
+ * minjail_copy_env: Copy an environment array (such as passed to execve),
+ * duplicating the environment strings and the array pointing at them.
+ *
+ * @env The environment to copy.
+ *
+ * Returns a pointer to the copied environment or NULL on memory allocation
+ * failure.
+ */
+char **minijail_copy_env(char *const *env);
+
+/*
+ * minjail_setenv: Set an environment variable in @env. Semantics match the
+ * standard setenv() function, but this operates on @env, not the global
+ * environment. @env must be dynamically allocated (as opposed to pointing to
+ * static data), e.g. via minijail_copy_env(). @name and @value get copied into
+ * newly-allocated memory.
+ *
+ * @env       Address of the environment to modify. Might be re-allocated to
+ *            make room for the new entry.
+ * @name      Name of the key to set.
+ * @value     The value to set.
+ * @overwrite Whether to replace the existing value for @name. If non-zero and
+ *            the entry is already present, no changes will be made.
+ *
+ * Returns 0 and modifies *@env on success, returns an error code otherwise.
+ */
+int minijail_setenv(char ***env, const char *name, const char *value,
+		    int overwrite);
+
 #ifdef __cplusplus
 }; /* extern "C" */
 #endif
diff --git a/util_unittest.cc b/util_unittest.cc
index c97dc2a..ab4805a 100644
--- a/util_unittest.cc
+++ b/util_unittest.cc
@@ -15,6 +15,20 @@
 
 #include "util.h"
 
+namespace {
+
+std::string dump_env(const char *const *env) {
+  std::string result;
+  for (; *env; ++env) {
+    result += *env;
+    result += "\n";
+  }
+
+  return result;
+}
+
+}  // namespace
+
 // Sanity check for the strip func.
 TEST(strip, basic) {
   char str[] = " foo\t";
@@ -81,3 +95,66 @@ TEST(tokenize, empty_tokens) {
   ASSERT_EQ(nullptr, p);
   ASSERT_EQ(nullptr, tokenize(&p, ","));
 }
+
+// Check environment manipulation functions.
+TEST(environment, copy_and_modify) {
+  minijail_free_env(nullptr);
+
+  char **env = minijail_copy_env(nullptr);
+  EXPECT_EQ("", dump_env(env));
+  minijail_free_env(env);
+
+  const char *const kConstEnv[] = {
+    "val1=1",
+    "val2=2",
+    "dup=1",
+    "dup=2",
+    "empty=",
+    nullptr,
+  };
+
+  // libc unfortunately uses char* const[] as the type for the environment, and
+  // we match that. It's actually missing a const-ness of the chars making up
+  // the environment strings, but we need that to initialize the |kEnv|
+  // constant. Hence, do a cast here to force things into alignment...
+  char* const* kEnv = const_cast<char* const*>(kConstEnv);
+
+  env = minijail_copy_env(kEnv);
+  EXPECT_EQ("val1=1\nval2=2\ndup=1\ndup=2\nempty=\n", dump_env(env));
+  minijail_free_env(env);
+
+  env = minijail_copy_env(kEnv);
+  EXPECT_EQ("val1=1\nval2=2\ndup=1\ndup=2\nempty=\n", dump_env(env));
+
+  EXPECT_EQ(EINVAL, minijail_setenv(nullptr, "val1", "3", 1));
+  char **env_ret = nullptr;
+  EXPECT_EQ(EINVAL, minijail_setenv(&env_ret, "val1", "3", 1));
+
+  env_ret = env;
+  EXPECT_EQ(EINVAL, minijail_setenv(&env_ret, nullptr, "3", 1));
+  EXPECT_EQ(env, env_ret);
+  EXPECT_EQ(EINVAL, minijail_setenv(&env_ret, "", "3", 1));
+  EXPECT_EQ(env, env_ret);
+  EXPECT_EQ(EINVAL, minijail_setenv(&env_ret, "", nullptr, 1));
+  EXPECT_EQ(env, env_ret);
+
+  EXPECT_EQ(0, minijail_setenv(&env, "val1", "3", 0));
+  EXPECT_EQ("val1=1\nval2=2\ndup=1\ndup=2\nempty=\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "val1", "3", 1));
+  EXPECT_EQ("val1=3\nval2=2\ndup=1\ndup=2\nempty=\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "val2", "4", 1));
+  EXPECT_EQ("val1=3\nval2=4\ndup=1\ndup=2\nempty=\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "dup", "5", 1));
+  EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "empty", "6", 1));
+  EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=6\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "empty", "", 1));
+  EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "new1", "7", 0));
+  EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\n", dump_env(env));
+  EXPECT_EQ(0, minijail_setenv(&env, "new2", "8", 1));
+  EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n",
+            dump_env(env));
+
+  minijail_free_env(env);
+}