Snap for 6877830 from 5f9e3001c61626d2863dad91248ba8496c3ef511 to sdk-releasesdk-release

Change-Id: I7e80e07458a224dc67d8d41d2c397e87e1834d4d

20 files changed, 598 insertions, 240 deletions

diff --git a/Android.bp b/Android.bp
index 162f0242..482f6c73 100644
--- a/Android.bp
+++ b/Android.bp
@@ -453,26 +453,44 @@ cc_binary {
     shared_libs: minijailCommonLibraries + ["libminijail"],
 }
 
+rust_defaults {
+    name: "libminijail_rust_defaults",
+    target: {
+        darwin: {
+            enabled: false,
+        },
+    },
+}
+
 // This target was generated by cargo2android.py --run --device, with some
 // manual fixes.
-rust_library_rlib {
+rust_library {
     name: "libminijail_sys",
+    defaults: ["libminijail_rust_defaults"],
     host_supported: true,
     crate_name: "minijail_sys",
     srcs: ["rust/minijail-sys/lib.rs"],
     edition: "2018",
-    rlibs: [
+    rustlibs: [
         "liblibc",
     ],
-    static_libs: [
-        "libminijail",
-    ],
     shared_libs: [
         "libcap",
+        "libminijail",
+    ],
+}
+
+// This target was generated by cargo2android.py --run --device, with some
+// manual fixes.
+rust_library {
+    name: "libminijail_rust",
+    defaults: ["libminijail_rust_defaults"],
+    host_supported: true,
+    crate_name: "minijail",
+    srcs: ["rust/minijail/src/lib.rs"],
+    edition: "2018",
+    rustlibs: [
+        "liblibc",
+        "libminijail_sys",
     ],
-    target: {
-        darwin: {
-            enabled: false,
-        },
-    },
 }
diff --git a/HACKING.md b/HACKING.md
index 3a9fcfcd..abdf5163 100644
--- a/HACKING.md
+++ b/HACKING.md
@@ -14,8 +14,7 @@ For local experimentation (using Minijail libraries from the source directory):
 
 ```
 $ make LIBDIR=/lib64
-$ sudo ./minijail0 --preload-library=./libminijailpreload.so \
-  -u ${USER} -g 5000 -- /usr/bin/id
+$ sudo ./minijail0.sh -u ${USER} -g 5000 -- /usr/bin/id
 ```
 
 For system-wide usage, install `libminijail.so` and `libminijailpreload.so` to
diff --git a/README.md b/README.md
index 914c8675..9c18c6aa 100644
--- a/README.md
+++ b/README.md
@@ -60,7 +60,7 @@ We've got a couple of contact points.
 The following talk serves as a good introduction to Minijail and how it can be used.
 
 [Video](https://drive.google.com/file/d/0BwPS_JpKyELWZTFBcTVsa1hhYjA/preview),
-[slides](https://docs.google.com/presentation/d/1r6LpvDZtYrsl7ryOV4HtpUR-phfCLRL6PA-chcL1Kno/present).
+[slides](https://docs.google.com/presentation/d/e/2PACX-1vRBqpin5xR9sng6lIBPjG0XQtu-uWWgr0ds-M3zW13XpDO-bTcMERLwoHUEB9078p1yqr9L-su9n5dk/pub).
 
 ## Example usage
 
diff --git a/bpf.c b/bpf.c
index e130850c..d8bf46cb 100644
--- a/bpf.c
+++ b/bpf.c
@@ -236,6 +236,11 @@ size_t bpf_arg_comp(struct sock_filter **pfilter, int op, int argidx,
 				unsigned char jt, unsigned char jf);
 	int flip = 0;
 
+	if (!filter) {
+		*pfilter = NULL;
+		return 0;
+	}
+
 	/* Load arg */
 	curr_block += bpf_load_arg(curr_block, argidx);
 
diff --git a/gen_constants-inl.h b/gen_constants-inl.h
index 1248254c..686ec5e9 100644
--- a/gen_constants-inl.h
+++ b/gen_constants-inl.h
@@ -1,6 +1,11 @@
+/* Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
 #if defined(__i386__) || defined(__x86_64__)
 #include <asm/prctl.h>
-#endif // __i386__ || __x86_64__
+#endif /* __i386__ || __x86_64__ */
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/fd.h>
@@ -23,8 +28,45 @@
 
 #include "arch.h"
 
-// These defines use C structures that are not defined in the same headers which
-// cause our CPP logic to fail w/undefined identifiers.  Remove them to avoid
-// build errors on such broken systems.
+/* These defines use C structures that are not defined in the same headers which
+ * cause our CPP logic to fail w/undefined identifiers.  Remove them to avoid
+ * build errors on such broken systems.
+ */
 #undef BLKTRACESETUP
 #undef FS_IOC_FIEMAP
+
+/* The old glibc bundled with the Android host toolchain is missing some ioctl
+ * definitions used by minijail policy in crosvm and other projects. Locally
+ * define them below.
+ * This UAPI is taken from sanitized bionic headers.
+ */
+
+/* <linux/fs.h> */
+#if !defined(FS_IOC_FSGETXATTR) && !defined(FS_IOC_FSSETXATTR)
+struct fsxattr {
+	__u32 fsx_xflags;
+	__u32 fsx_extsize;
+	__u32 fsx_nextents;
+	__u32 fsx_projid;
+	__u32 fsx_cowextsize;
+	unsigned char fsx_pad[8];
+};
+#define FS_IOC_FSGETXATTR _IOR('X', 31, struct fsxattr)
+#define FS_IOC_FSSETXATTR _IOW('X', 32, struct fsxattr)
+#endif /* !FS_IOC_FSGETXATTR && !FS_IOC_FSSETXATTR */
+
+/* <linux/fscrypt.h> */
+#if !defined(FS_IOC_SET_ENCRYPTION_POLICY) &&                                  \
+    !defined(FS_IOC_GET_ENCRYPTION_POLICY)
+#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+struct fscrypt_policy_v1 {
+	__u8 version;
+	__u8 contents_encryption_mode;
+	__u8 filenames_encryption_mode;
+	__u8 flags;
+	__u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+};
+#define fscrypt_policy fscrypt_policy_v1
+#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
+#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
+#endif /* !FS_IOC_SET_ENCRYPTION_POLICY && !FS_IOC_GET_ENCRYPTION_POLICY */
diff --git a/gen_syscalls-inl.h b/gen_syscalls-inl.h
new file mode 100644
index 00000000..6203ae47
--- /dev/null
+++ b/gen_syscalls-inl.h
@@ -0,0 +1,63 @@
+/* Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include <asm/unistd.h>
+
+/* Ideally minijail is compiled against a modern libc, which has modern copies
+ * of Linux uapi for ioctls, and unistd.h for syscalls. However, sometimes this
+ * isn't possible - such as when building with the Android host toolchain - so
+ * locally define the system calls in use in active seccomp policy files.
+ * This UAPI is taken from sanitized bionic headers.
+ */
+
+#ifndef __NR_copy_file_range
+#ifdef __x86_64__
+#define __NR_copy_file_range 326
+#elif __i386__
+#define __NR_copy_file_range 377
+#elif __arm64__
+#define __NR_copy_file_range 285
+#endif
+#endif /* __NR_copy_file_range */
+
+#ifndef __NR_getrandom
+#ifdef __x86_64__
+#define __NR_getrandom 318
+#elif __i386__
+#define __NR_getrandom 355
+#elif __arm64__
+#define __NR_getrandom 278
+#endif
+#endif /* __NR_getrandom */
+
+#ifndef __NR_memfd_create
+#ifdef __x86_64__
+#define __NR_memfd_create 319
+#elif __i386__
+#define __NR_memfd_create 356
+#elif __arm64__
+#define __NR_memfd_create 279
+#endif
+#endif /* __NR_memfd_create */
+
+#ifndef __NR_renameat2
+#ifdef __x86_64__
+#define __NR_renameat2 316
+#elif __i386__
+#define __NR_renameat2 353
+#elif __arm64__
+#define __NR_renameat2 276
+#endif
+#endif /* __NR_renameat2 */
+
+#ifndef __NR_statx
+#ifdef __x86_64__
+#define __NR_statx 332
+#elif __i386__
+#define __NR_statx 383
+#elif __arm64__
+#define __NR_statx 291
+#endif
+#endif /* __NR_statx */
diff --git a/gen_syscalls.sh b/gen_syscalls.sh
index 43e39b7f..ca26322c 100755
--- a/gen_syscalls.sh
+++ b/gen_syscalls.sh
@@ -48,7 +48,7 @@ SED_MULTILINE='s/#define __(ARM_)?(NR_)([[:lower:]0-9_]*) (.*)$/#ifdef __\1\2\3\
 cat <<-EOF > "${OUTFILE}"
 /* GENERATED BY MAKEFILE */
 #include <stddef.h>
-#include <asm/unistd.h>
+#include "gen_syscalls-inl.h"
 #include "libsyscalls.h"
 const struct syscall_entry syscall_table[] = {
 $(${BUILD} | sed -Ene "${SED_MULTILINE}")
diff --git a/libminijail.c b/libminijail.c
index f42ac2f5..abe5a78b 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -313,7 +313,9 @@ void minijail_preexec(struct minijail *j)
 struct minijail API *minijail_new(void)
 {
 	struct minijail *j = calloc(1, sizeof(struct minijail));
-	j->remount_mode = MS_PRIVATE;
+	if (j) {
+		j->remount_mode = MS_PRIVATE;
+	}
 	return j;
 }
 
@@ -722,11 +724,6 @@ char API *minijail_get_original_path(struct minijail *j,
 	return strdup(path_inside_chroot);
 }
 
-size_t minijail_get_tmpfs_size(const struct minijail *j)
-{
-	return j->tmpfs_size;
-}
-
 void API minijail_mount_dev(struct minijail *j)
 {
 	j->flags.mount_dev = 1;
@@ -1150,15 +1147,16 @@ struct marshal_state {
 	char *buf;
 };
 
-void marshal_state_init(struct marshal_state *state, char *buf,
-			size_t available)
+static void marshal_state_init(struct marshal_state *state, char *buf,
+			       size_t available)
 {
 	state->available = available;
 	state->buf = buf;
 	state->total = 0;
 }
 
-void marshal_append(struct marshal_state *state, void *src, size_t length)
+static void marshal_append(struct marshal_state *state, const void *src,
+			   size_t length)
 {
 	size_t copy_len = MIN(state->available, length);
 
@@ -1172,7 +1170,13 @@ void marshal_append(struct marshal_state *state, void *src, size_t length)
 	state->total += length;
 }
 
-void marshal_mount(struct marshal_state *state, const struct mountpoint *m)
+static void marshal_append_string(struct marshal_state *state, const char *src)
+{
+	marshal_append(state, src, strlen(src) + 1);
+}
+
+static void marshal_mount(struct marshal_state *state,
+			  const struct mountpoint *m)
 {
 	marshal_append(state, m->src, strlen(m->src) + 1);
 	marshal_append(state, m->dest, strlen(m->dest) + 1);
@@ -1183,23 +1187,23 @@ void marshal_mount(struct marshal_state *state, const struct mountpoint *m)
 	marshal_append(state, (char *)&m->flags, sizeof(m->flags));
 }
 
-void minijail_marshal_helper(struct marshal_state *state,
-			     const struct minijail *j)
+static void minijail_marshal_helper(struct marshal_state *state,
+				    const struct minijail *j)
 {
 	struct mountpoint *m = NULL;
 	size_t i;
 
 	marshal_append(state, (char *)j, sizeof(*j));
 	if (j->user)
-		marshal_append(state, j->user, strlen(j->user) + 1);
+		marshal_append_string(state, j->user);
 	if (j->suppl_gid_list) {
 		marshal_append(state, j->suppl_gid_list,
 			       j->suppl_gid_count * sizeof(gid_t));
 	}
 	if (j->chrootdir)
-		marshal_append(state, j->chrootdir, strlen(j->chrootdir) + 1);
+		marshal_append_string(state, j->chrootdir);
 	if (j->hostname)
-		marshal_append(state, j->hostname, strlen(j->hostname) + 1);
+		marshal_append_string(state, j->hostname);
 	if (j->alt_syscall_table) {
 		marshal_append(state, j->alt_syscall_table,
 			       strlen(j->alt_syscall_table) + 1);
@@ -1213,7 +1217,7 @@ void minijail_marshal_helper(struct marshal_state *state,
 		marshal_mount(state, m);
 	}
 	for (i = 0; i < j->cgroup_count; ++i)
-		marshal_append(state, j->cgroups[i], strlen(j->cgroups[i]) + 1);
+		marshal_append_string(state, j->cgroups[i]);
 }
 
 size_t API minijail_size(const struct minijail *j)
@@ -2209,8 +2213,8 @@ void API minijail_enter(const struct minijail *j)
 		if (j->remount_mode) {
 			if (mount(NULL, "/", NULL, MS_REC | j->remount_mode,
 				  NULL))
-				pdie("mount(NULL, /, NULL, MS_REC | MS_PRIVATE,"
-				     " NULL) failed");
+				pdie("mount(NULL, /, NULL, "
+				     "MS_REC | j->remount_mode, NULL) failed");
 		}
 	}
 
@@ -2334,12 +2338,12 @@ void API minijail_enter(const struct minijail *j)
 /* TODO(wad): will visibility affect this variable? */
 static int init_exitstatus = 0;
 
-void init_term(int sig attribute_unused)
+static void init_term(int sig attribute_unused)
 {
 	_exit(init_exitstatus);
 }
 
-void init(pid_t rootpid)
+static void init(pid_t rootpid)
 {
 	pid_t pid;
 	int status;
diff --git a/libminijail_unittest.cc b/libminijail_unittest.cc
index c6cc0c84..23a30b75 100644
--- a/libminijail_unittest.cc
+++ b/libminijail_unittest.cc
@@ -95,9 +95,6 @@ std::map<std::string, std::string> GetNamespaces(
 
 }  // namespace
 
-/* Prototypes needed only by test. */
-size_t minijail_get_tmpfs_size(const struct minijail *);
-
 /* Silence unused variable warnings. */
 TEST(silence, silence_unused) {
   EXPECT_STREQ(kLdPreloadEnvVar, kLdPreloadEnvVar);
@@ -1050,58 +1047,6 @@ TEST_F(NamespaceTest, test_enter_ns) {
   }
 }
 
-TEST(Test, parse_size) {
-  size_t size;
-
-  ASSERT_EQ(0, parse_size(&size, "42"));
-  ASSERT_EQ(42U, size);
-
-  ASSERT_EQ(0, parse_size(&size, "16K"));
-  ASSERT_EQ(16384U, size);
-
-  ASSERT_EQ(0, parse_size(&size, "1M"));
-  ASSERT_EQ(1024U * 1024, size);
-
-  uint64_t gigabyte = 1024ULL * 1024 * 1024;
-  ASSERT_EQ(0, parse_size(&size, "3G"));
-  ASSERT_EQ(3U, size / gigabyte);
-  ASSERT_EQ(0U, size % gigabyte);
-
-  ASSERT_EQ(0, parse_size(&size, "4294967294"));
-  ASSERT_EQ(3U, size / gigabyte);
-  ASSERT_EQ(gigabyte - 2, size % gigabyte);
-
-#if __WORDSIZE == 64
-  uint64_t exabyte = gigabyte * 1024 * 1024 * 1024;
-  ASSERT_EQ(0, parse_size(&size, "9E"));
-  ASSERT_EQ(9U, size / exabyte);
-  ASSERT_EQ(0U, size % exabyte);
-
-  ASSERT_EQ(0, parse_size(&size, "15E"));
-  ASSERT_EQ(15U, size / exabyte);
-  ASSERT_EQ(0U, size % exabyte);
-
-  ASSERT_EQ(0, parse_size(&size, "18446744073709551614"));
-  ASSERT_EQ(15U, size / exabyte);
-  ASSERT_EQ(exabyte - 2, size % exabyte);
-
-  ASSERT_EQ(-ERANGE, parse_size(&size, "16E"));
-  ASSERT_EQ(-ERANGE, parse_size(&size, "19E"));
-  ASSERT_EQ(-EINVAL, parse_size(&size, "7GTPE"));
-#elif __WORDSIZE == 32
-  ASSERT_EQ(-ERANGE, parse_size(&size, "5G"));
-  ASSERT_EQ(-ERANGE, parse_size(&size, "9G"));
-  ASSERT_EQ(-ERANGE, parse_size(&size, "9E"));
-  ASSERT_EQ(-ERANGE, parse_size(&size, "7GTPE"));
-#endif
-
-  ASSERT_EQ(-EINVAL, parse_size(&size, ""));
-  ASSERT_EQ(-EINVAL, parse_size(&size, "14u"));
-  ASSERT_EQ(-EINVAL, parse_size(&size, "14.2G"));
-  ASSERT_EQ(-EINVAL, parse_size(&size, "-1G"));
-  ASSERT_EQ(-EINVAL, parse_size(&size, "; /bin/rm -- "));
-}
-
 void TestCreateSession(bool create_session) {
   int status;
   int pipe_fds[2];
diff --git a/libminijailpreload.c b/libminijailpreload.c
index a1e376e9..a98b736d 100644
--- a/libminijailpreload.c
+++ b/libminijailpreload.c
@@ -71,6 +71,7 @@ static int fake_main(int argc, char **argv, char **envp)
 		die("preload: failed to parse minijail from parent");
 	close(fd);
 
+	unset_in_env(envp, kFdEnvVar);
 	/* TODO(ellyjones): this trashes existing preloads, so one can't do:
 	 * LD_PRELOAD="/tmp/test.so libminijailpreload.so" prog; the
 	 * descendants of prog will have no LD_PRELOAD set at all.
diff --git a/linux-x86/libsyscalls.gen.c b/linux-x86/libsyscalls.gen.c
index c33e1243..34af6ca9 100644
--- a/linux-x86/libsyscalls.gen.c
+++ b/linux-x86/libsyscalls.gen.c
@@ -1,6 +1,6 @@
 /* GENERATED BY MAKEFILE */
 #include <stddef.h>
-#include <asm/unistd.h>
+#include "gen_syscalls-inl.h"
 #include "libsyscalls.h"
 const struct syscall_entry syscall_table[] = {
 #ifdef __NR_read
diff --git a/minijail0.1 b/minijail0.1
index 820d3ca0..a3f8c9bc 100644
--- a/minijail0.1
+++ b/minijail0.1
@@ -12,12 +12,14 @@ Runs PROGRAM inside a sandbox.
 Run using the alternate syscall table named \fItable\fR. Only available on kernels
 and architectures that support the \fBPR_ALT_SYSCALL\fR option of \fBprctl\fR(2).
 .TP
-\fB-b <src>[,<dest>[,<writeable>]]
+\fB-b <src>[,[dest][,<writeable>]]
 Bind-mount \fIsrc\fR into the chroot directory at \fIdest\fR, optionally writeable.
 The \fIsrc\fR path must be an absolute path.
+
 If \fIdest\fR is not specified, it will default to \fIsrc\fR.
 If the destination does not exist, it will be created as a file or directory
 based on the \fIsrc\fR type (including missing parent directories).
+
 To create a writable bind-mount set \fIwritable\fR to \fB1\fR. If not specified
 it will default to \fB0\fR (read-only).
 .TP
@@ -134,22 +136,22 @@ If the destination does not exist, it will be created as a directory (including
 missing parent directories).
 .TP
 \fB-K[mode]\fR
-Don't mark all existing mounts as MS_PRIVATE.
+Don't mark all existing mounts as MS_SLAVE.
 This option is \fBdangerous\fR as it negates most of the functionality of \fB-v\fR.
 You very likely don't need this.
 
 You may specify a mount propagation mode in which case, that will be used
-instead of the default MS_PRIVATE.  See the \fBmount\fR(2) man page and the
+instead of the default MS_SLAVE.  See the \fBmount\fR(2) man page and the
 kernel docs \fIDocumentation/filesystems/sharedsubtree.txt\fR for more
 technical details, but a brief guide:
 
 .IP
 \[bu] \fBslave\fR Changes in the parent mount namespace will propagate in, but
 changes in this mount namespace will not propagate back out.  This is usually
-what people want to use.
+what people want to use, and is the default behavior if you don't specify \fB-K\fR.
 .IP
 \[bu] \fBprivate\fR No changes in either mount namespace will propagate.
-This is the default behavior if you don't specify \fB-K\fR.
+This provides the most isolation.
 .IP
 \[bu] \fBshared\fR Changes in the parent and this mount namespace will freely
 propagate back and forth.  This is not recommended.
@@ -252,8 +254,8 @@ Change users to the specified \fIuser\fR name, or numeric user ID \fIuid\fR.
 Enter a new user namespace (implies \fB-p\fR).
 .TP
 \fB-v\fR
-Run inside a new VFS namespace. This option makes the program's mountpoints
-independent of the rest of the system's.
+Run inside a new VFS namespace. This option prevents mounts performed by the
+program from affecting the rest of the system (but see \fB-K\fR).
 .TP
 \fB-V <file>\fR
 Enter the VFS namespace specified by \fIfile\fR.
diff --git a/minijail0.sh b/minijail0.sh
new file mode 100755
index 00000000..cd5303a6
--- /dev/null
+++ b/minijail0.sh
@@ -0,0 +1,9 @@
+#!/bin/sh
+# Copyright 2020 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# Helper for running minijail0 in a compiled checkout.
+
+dir="$(dirname "$0")"
+exec "${dir}/minijail0" --preload-library="${dir}/libminijailpreload.so" "$@"
diff --git a/minijail0_cli.c b/minijail0_cli.c
index c3da5de9..94d8578b 100644
--- a/minijail0_cli.c
+++ b/minijail0_cli.c
@@ -29,6 +29,30 @@
 #define IDMAP_LEN 32U
 #define DEFAULT_TMP_SIZE (64 * 1024 * 1024)
 
+/*
+ * A malloc() that aborts on failure.  We only implement this in the CLI as
+ * the library should return ENOMEM errors when allocations fail.
+ */
+static void *xmalloc(size_t size)
+{
+	void *ret = malloc(size);
+	if (!ret) {
+		perror("malloc() failed");
+		exit(1);
+	}
+	return ret;
+}
+
+static char *xstrdup(const char *s)
+{
+	char *ret = strdup(s);
+	if (!ret) {
+		perror("strdup() failed");
+		exit(1);
+	}
+	return ret;
+}
+
 static void set_user(struct minijail *j, const char *arg, uid_t *out_uid,
 		     gid_t *out_gid)
 {
@@ -289,7 +313,7 @@ static void add_mount(struct minijail *j, char *arg)
 static char *build_idmap(id_t id, id_t lowerid)
 {
 	int ret;
-	char *idmap = malloc(IDMAP_LEN);
+	char *idmap = xmalloc(IDMAP_LEN);
 	ret = snprintf(idmap, IDMAP_LEN, "%d %d 1", id, lowerid);
 	if (ret < 0 || (size_t)ret >= IDMAP_LEN) {
 		free(idmap);
@@ -487,12 +511,7 @@ static void read_seccomp_filter(const char *filter_path,
 	rewind(f);
 
 	filter->len = filter_size / sizeof(struct sock_filter);
-	filter->filter = malloc(filter_size);
-	if (!filter->filter) {
-		fclose(f);
-		fprintf(stderr, "failed to allocate memory for filter: %m");
-		exit(1);
-	}
+	filter->filter = xmalloc(filter_size);
 	if (fread(filter->filter, sizeof(struct sock_filter), filter->len, f) !=
 	    filter->len) {
 		fclose(f);
@@ -508,7 +527,7 @@ static void usage(const char *progn)
 	/* clang-format off */
 	printf("Usage: %s [-dGhHiIKlLnNprRstUvyYz]\n"
 	       "  [-a <table>]\n"
-	       "  [-b <src>[,<dest>[,<writeable>]]] [-k <src>,<dest>,<type>[,<flags>[,<data>]]]\n"
+	       "  [-b <src>[,[dest][,<writeable>]]] [-k <src>,<dest>,<type>[,<flags>[,<data>]]]\n"
 	       "  [-c <caps>] [-C <dir>] [-P <dir>] [-e[file]] [-f <file>] [-g <group>]\n"
 	       "  [-m[<uid> <loweruid> <count>]*] [-M[<gid> <lowergid> <count>]*] [--profile <name>]\n"
 	       "  [-R <type,cur,max>] [-S <file>] [-t[size]] [-T <type>] [-u <user>] [-V <file>]\n"
@@ -626,6 +645,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[],
 	int binding = 0;
 	int chroot = 0, pivot_root = 0;
 	int mount_ns = 0, change_remount = 0;
+	const char *remount_mode = NULL;
 	int inherit_suppl_gids = 0, keep_suppl_gids = 0;
 	int caps = 0, ambient_caps = 0;
 	int seccomp = -1;
@@ -727,11 +747,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[],
 			add_mount(j, optarg);
 			break;
 		case 'K':
-			if (optarg) {
-				set_remount_mode(j, optarg);
-			} else {
-				minijail_skip_remount_private(j);
-			}
+			remount_mode = optarg;
 			change_remount = 1;
 			break;
 		case 'P':
@@ -761,6 +777,37 @@ int parse_args(struct minijail *j, int argc, char *const argv[],
 			break;
 		case 'v':
 			minijail_namespace_vfs(j);
+			/*
+			 * Set the default mount propagation in the command-line
+			 * tool to MS_SLAVE.
+			 *
+			 * When executing the sandboxed program in a new mount
+			 * namespace the Minijail library will by default
+			 * remount all mounts with the MS_PRIVATE flag. While
+			 * this is an appropriate, safe default for the library,
+			 * MS_PRIVATE can be problematic: unmount events will
+			 * not propagate into mountpoints marked as MS_PRIVATE.
+			 * This means that if a mount is unmounted in the root
+			 * mount namespace, it will not be unmounted in the
+			 * non-root mount namespace.
+			 * This in turn can be problematic because activity in
+			 * the non-root mount namespace can now directly
+			 * influence the root mount namespace (e.g. preventing
+			 * re-mounts of said mount), which would be a privilege
+			 * inversion.
+			 *
+			 * Setting the default in the command-line to MS_SLAVE
+			 * will still prevent mounts from leaking out of the
+			 * non-root mount namespace but avoid these
+			 * privilege-inversion issues.
+			 * For cases where mounts should not flow *into* the
+			 * namespace either, the user can pass -Kprivate.
+			 * Note that mounts are marked as MS_PRIVATE by default
+			 * by the kernel, so unless the init process (like
+			 * systemd) or something else marks them as shared, this
+			 * won't do anything.
+			 */
+			minijail_remount_mode(j, MS_SLAVE);
 			mount_ns = 1;
 			break;
 		case 'V':
@@ -820,7 +867,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[],
 				uidmap = NULL;
 			}
 			if (optarg)
-				uidmap = strdup(optarg);
+				uidmap = xstrdup(optarg);
 			break;
 		case 'M':
 			set_gidmap = 1;
@@ -829,7 +876,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[],
 				gidmap = NULL;
 			}
 			if (optarg)
-				gidmap = strdup(optarg);
+				gidmap = xstrdup(optarg);
 			break;
 		case 'a':
 			if (0 != minijail_use_alt_syscall(j, optarg)) {
@@ -971,6 +1018,15 @@ int parse_args(struct minijail *j, int argc, char *const argv[],
 		exit(1);
 	}
 
+	/* Configure the remount flag here to avoid having -v override it. */
+	if (change_remount) {
+		if (remount_mode != NULL) {
+			set_remount_mode(j, remount_mode);
+		} else {
+			minijail_skip_remount_private(j);
+		}
+	}
+
 	/*
 	 * Proceed in setting the supplementary gids specified on the
 	 * cmdline options.
diff --git a/rust/minijail/Cargo.toml b/rust/minijail/Cargo.toml
index 83157e4d..6cf9837d 100644
--- a/rust/minijail/Cargo.toml
+++ b/rust/minijail/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "minijail"
-version = "0.2.0"
+version = "0.2.1"
 description = "Provides a safe Rust friendly interface to libminijail."
 authors = ["The Chromium OS Authors"]
 edition = "2018"
diff --git a/rust/minijail/src/lib.rs b/rust/minijail/src/lib.rs
index 66001eae..03ef8780 100644
--- a/rust/minijail/src/lib.rs
+++ b/rust/minijail/src/lib.rs
@@ -72,6 +72,17 @@ pub enum Error {
     WrongProgramAlignment,
     /// File size should be non-zero and a multiple of sock_filter
     WrongProgramSize,
+
+    /// The command was not found.
+    NoCommand,
+    /// The command could not be run.
+    NoAccess,
+    /// Process was killed by SIGSYS indicating a seccomp violation.
+    SeccompViolation(i32),
+    /// Process was killed by a signal other than SIGSYS.
+    Killed(u8),
+    /// Process finished returning a non-zero code.
+    ReturnCode(u8),
 }
 
 impl Display for Error {
@@ -155,6 +166,11 @@ impl Display for Error {
                 "the alignment of bpf file was not a multiple of that of sock_filter"
             ),
             WrongProgramSize => write!(f, "bpf file was empty or not a multiple of sock_filter"),
+            NoCommand => write!(f, "command was not found"),
+            NoAccess => write!(f, "unable to execute command"),
+            SeccompViolation(s) => write!(f, "seccomp violation syscall #{}", s),
+            Killed(s) => write!(f, "killed with signal number {}", s),
+            ReturnCode(e) => write!(f, "exited with code {}", e),
         }
     }
 }
@@ -209,6 +225,11 @@ pub struct Minijail {
     jail: *mut minijail,
 }
 
+#[link(name = "c")]
+extern "C" {
+    fn __libc_current_sigrtmax() -> libc::c_int;
+}
+
 impl Minijail {
     /// Creates a new jail configuration.
     pub fn new() -> Result<Minijail> {
@@ -237,6 +258,21 @@ impl Minijail {
             minijail_change_gid(self.jail, gid);
         }
     }
+    pub fn change_user(&mut self, user: &str) -> Result<()> {
+        let user_cstring = CString::new(user).map_err(|_| Error::StrToCString(user.to_owned()))?;
+        unsafe {
+            minijail_change_user(self.jail, user_cstring.as_ptr());
+        }
+        Ok(())
+    }
+    pub fn change_group(&mut self, group: &str) -> Result<()> {
+        let group_cstring =
+            CString::new(group).map_err(|_| Error::StrToCString(group.to_owned()))?;
+        unsafe {
+            minijail_change_group(self.jail, group_cstring.as_ptr());
+        }
+        Ok(())
+    }
     pub fn set_supplementary_gids(&mut self, ids: &[libc::gid_t]) {
         unsafe {
             minijail_set_supplementary_gids(self.jail, ids.len(), ids.as_ptr());
@@ -315,7 +351,7 @@ impl Minijail {
         let pathstring = path
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(path.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(path.to_owned()))?;
         let filename =
             CString::new(pathstring).map_err(|_| Error::PathToCString(path.to_owned()))?;
         unsafe {
@@ -443,7 +479,7 @@ impl Minijail {
         let pathstring = dir
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(dir.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(dir.to_owned()))?;
         let dirname = CString::new(pathstring).map_err(|_| Error::PathToCString(dir.to_owned()))?;
         let ret = unsafe { minijail_enter_chroot(self.jail, dirname.as_ptr()) };
         if ret < 0 {
@@ -455,7 +491,7 @@ impl Minijail {
         let pathstring = dir
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(dir.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(dir.to_owned()))?;
         let dirname = CString::new(pathstring).map_err(|_| Error::PathToCString(dir.to_owned()))?;
         let ret = unsafe { minijail_enter_pivot_root(self.jail, dirname.as_ptr()) };
         if ret < 0 {
@@ -477,12 +513,12 @@ impl Minijail {
         let src_os = src
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(src.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(src.to_owned()))?;
         let src_path = CString::new(src_os).map_err(|_| Error::StrToCString(src_os.to_owned()))?;
         let dest_os = dest
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(dest.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(dest.to_owned()))?;
         let dest_path =
             CString::new(dest_os).map_err(|_| Error::StrToCString(dest_os.to_owned()))?;
         let fstype_string =
@@ -529,12 +565,12 @@ impl Minijail {
         let src_os = src
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(src.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(src.to_owned()))?;
         let src_path = CString::new(src_os).map_err(|_| Error::StrToCString(src_os.to_owned()))?;
         let dest_os = dest
             .as_os_str()
             .to_str()
-            .ok_or(Error::PathToCString(dest.to_owned()))?;
+            .ok_or_else(|| Error::PathToCString(dest.to_owned()))?;
         let dest_path =
             CString::new(dest_os).map_err(|_| Error::StrToCString(dest_os.to_owned()))?;
         let ret = unsafe {
@@ -578,7 +614,9 @@ impl Minijail {
         inheritable_fds: &[(RawFd, RawFd)],
         args: &[&str],
     ) -> Result<pid_t> {
-        let cmd_os = cmd.to_str().ok_or(Error::PathToCString(cmd.to_owned()))?;
+        let cmd_os = cmd
+            .to_str()
+            .ok_or_else(|| Error::PathToCString(cmd.to_owned()))?;
         let cmd_cstr = CString::new(cmd_os).map_err(|_| Error::StrToCString(cmd_os.to_owned()))?;
 
         // Converts each incoming `args` string to a `CString`, and then puts each `CString` pointer
@@ -695,6 +733,34 @@ impl Minijail {
         }
         Ok(ret as pid_t)
     }
+
+    pub fn wait(&self) -> Result<()> {
+        let ret: libc::c_int;
+        // This is safe because it does not modify the struct.
+        unsafe {
+            ret = minijail_wait(self.jail);
+        }
+        if ret == 0 {
+            return Ok(());
+        }
+        if ret == MINIJAIL_ERR_NO_COMMAND as libc::c_int {
+            return Err(Error::NoCommand);
+        }
+        if ret == MINIJAIL_ERR_NO_ACCESS as libc::c_int {
+            return Err(Error::NoAccess);
+        }
+        let sig_base: libc::c_int = MINIJAIL_ERR_SIG_BASE as libc::c_int;
+        let sig_max_code: libc::c_int = unsafe { __libc_current_sigrtmax() } + sig_base;
+        if ret > sig_base && ret <= sig_max_code {
+            return Err(Error::Killed(
+                (ret - MINIJAIL_ERR_SIG_BASE as libc::c_int) as u8,
+            ));
+        }
+        if ret > 0 && ret <= 0xff {
+            return Err(Error::ReturnCode(ret as u8));
+        }
+        unreachable!();
+    }
 }
 
 impl Drop for Minijail {
@@ -728,6 +794,8 @@ mod tests {
 
     use super::*;
 
+    const SHELL: &str = "/bin/sh";
+
     #[test]
     fn create_and_free() {
         unsafe {
@@ -774,6 +842,59 @@ mod tests {
         }
     }
 
+    macro_rules! expect_result {
+        ($call:expr, $expected:pat) => {
+            let got = $call;
+            match got {
+                $expected => {}
+                _ => {
+                    panic!("got {:?} expected {:?}", got, stringify!($expected));
+                }
+            }
+        };
+    }
+
+    #[test]
+    fn wait_success() {
+        let j = Minijail::new().unwrap();
+        j.run(Path::new("/bin/true"), &[1, 2], &[]).unwrap();
+        expect_result!(j.wait(), Ok(()));
+    }
+
+    #[test]
+    fn wait_killed() {
+        let j = Minijail::new().unwrap();
+        j.run(
+            Path::new(SHELL),
+            &[1, 2],
+            &[SHELL, "-c", "kill -9 $$ &\n/usr/bin/sleep 5"],
+        )
+        .unwrap();
+        expect_result!(j.wait(), Err(Error::Killed(9)));
+    }
+
+    #[test]
+    fn wait_returncode() {
+        let j = Minijail::new().unwrap();
+        j.run(Path::new("/bin/false"), &[1, 2], &[]).unwrap();
+        expect_result!(j.wait(), Err(Error::ReturnCode(1)));
+    }
+
+    #[test]
+    fn wait_noaccess() {
+        let j = Minijail::new().unwrap();
+        j.run(Path::new("/dev/null"), &[1, 2], &[]).unwrap();
+        expect_result!(j.wait(), Err(Error::NoAccess));
+    }
+
+    #[test]
+    fn wait_nocommand() {
+        let j = Minijail::new().unwrap();
+        j.run(Path::new("/bin/does not exist"), &[1, 2], &[])
+            .unwrap();
+        expect_result!(j.wait(), Err(Error::NoCommand));
+    }
+
     #[test]
     #[ignore] // privileged operation.
     fn chroot() {
diff --git a/syscall_filter.c b/syscall_filter.c
index 2c389aea..9cb3baf4 100644
--- a/syscall_filter.c
+++ b/syscall_filter.c
@@ -552,6 +552,10 @@ static ssize_t getmultiline(char **lineptr, size_t *n, FILE *stream)
 	/* Merge the lines. */
 	*n = ret + next_ret + 2;
 	line = realloc(line, *n);
+	if (!line) {
+		free(next_line);
+		return -1;
+	}
 	line[ret] = ' ';
 	memcpy(&line[ret + 1], next_line, next_ret + 1);
 	free(next_line);
@@ -815,6 +819,8 @@ int compile_filter(const char *filename, FILE *initial_file,
 
 	struct sock_filter *final_filter =
 	    calloc(final_filter_len, sizeof(struct sock_filter));
+	if (!final_filter)
+		die("could not allocate final BPF filter");
 
 	if (flatten_block_list(head, final_filter, 0, final_filter_len) < 0) {
 		free(final_filter);
diff --git a/syscall_filter_unittest.cc b/syscall_filter_unittest.cc
index 771dced3..8ead46c2 100644
--- a/syscall_filter_unittest.cc
+++ b/syscall_filter_unittest.cc
@@ -88,120 +88,6 @@ struct filter_block* test_compile_policy_line(
 
 }  // namespace
 
-TEST(util, parse_constant_unsigned) {
-  char *end;
-  long int c = 0;
-  std::string constant;
-
-#if defined(BITS32)
-  constant = "0x80000000";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(0x80000000U, static_cast<unsigned long int>(c));
-
-#elif defined(BITS64)
-  constant = "0x8000000000000000";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(0x8000000000000000UL, static_cast<unsigned long int>(c));
-#endif
-}
-
-TEST(util, parse_constant_unsigned_toobig) {
-  char *end;
-  long int c = 0;
-  std::string constant;
-
-#if defined(BITS32)
-  constant = "0x100000000";  // Too big for 32-bit unsigned long int.
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  // Error case should return 0.
-  EXPECT_EQ(0, c);
-
-#elif defined(BITS64)
-  constant = "0x10000000000000000";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  // Error case should return 0.
-  EXPECT_EQ(0, c);
-#endif
-}
-
-TEST(util, parse_constant_signed) {
-  char *end;
-  long int c = 0;
-  std::string constant = "-1";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(-1, c);
-}
-
-TEST(util, parse_constant_signed_toonegative) {
-  char *end;
-  long int c = 0;
-  std::string constant;
-
-#if defined(BITS32)
-  constant = "-0x80000001";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  // Error case should return 0.
-  EXPECT_EQ(0, c);
-
-#elif defined(BITS64)
-  constant = "-0x8000000000000001";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  // Error case should return 0.
-  EXPECT_EQ(0, c);
-#endif
-}
-
-TEST(util, parse_constant_complements) {
-  char* end;
-  long int c = 0;
-  std::string constant;
-
-#if defined(BITS32)
-  constant = "~0x005AF0FF|~0xFFA50FFF";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(c, 0xFFFFFF00);
-  constant = "0x0F|~(0x005AF000|0x00A50FFF)|0xF0";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(c, 0xFF0000FF);
-
-#elif defined(BITS64)
-  constant = "~0x00005A5AF0F0FFFF|~0xFFFFA5A50F0FFFFF";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(c, 0xFFFFFFFFFFFF0000UL);
-  constant = "0x00FF|~(0x00005A5AF0F00000|0x0000A5A50F0FFFFF)|0xFF00";
-  c = parse_constant(const_cast<char*>(constant.data()), &end);
-  EXPECT_EQ(c, 0xFFFF00000000FFFFUL);
-#endif
-}
-
-TEST(util, parse_parenthesized_expresions) {
-  char* end;
-
-  const std::vector<const char*> bad_expressions = {
-      "(1", "1)", "(1)1", "|(1)", "(1)|", "()",
-      "(",  "((", "(()",  "(()1", "1(0)",
-  };
-  for (const auto* expression : bad_expressions) {
-    std::string mutable_expression = expression;
-    long int c =
-        parse_constant(const_cast<char*>(mutable_expression.data()), &end);
-    EXPECT_EQ(reinterpret_cast<const void*>(end),
-              reinterpret_cast<const void*>(mutable_expression.data()));
-    // Error case should return 0.
-    EXPECT_EQ(c, 0) << "For expression: \"" << expression << "\"";
-  }
-
-  const std::vector<const char*> good_expressions = {
-      "(3)", "(1)|2", "1|(2)", "(1)|(2)", "((3))", "0|(1|2)", "(0|1|2)",
-  };
-  for (const auto* expression : good_expressions) {
-    std::string mutable_expression = expression;
-    long int c =
-        parse_constant(const_cast<char*>(mutable_expression.data()), &end);
-    EXPECT_EQ(c, 3) << "For expression: \"" << expression << "\"";
-  }
-}
-
 /* Test that setting one BPF instruction works. */
 TEST(bpf, set_bpf_instr) {
   struct sock_filter instr;
diff --git a/util.c b/util.c
index 48c3059e..7cee515d 100644
--- a/util.c
+++ b/util.c
@@ -434,7 +434,8 @@ char *path_join(const char *external_path, const char *internal_path)
 	/* One extra char for '/' and one for '\0', hence + 2. */
 	pathlen = strlen(external_path) + strlen(internal_path) + 2;
 	path = malloc(pathlen);
-	snprintf(path, pathlen, "%s/%s", external_path, internal_path);
+	if (path)
+		snprintf(path, pathlen, "%s/%s", external_path, internal_path);
 
 	return path;
 }
diff --git a/util_unittest.cc b/util_unittest.cc
index ab4805a9..35a99e5d 100644
--- a/util_unittest.cc
+++ b/util_unittest.cc
@@ -13,6 +13,7 @@
 
 #include <gtest/gtest.h>
 
+#include "bpf.h"
 #include "util.h"
 
 namespace {
@@ -158,3 +159,202 @@ TEST(environment, copy_and_modify) {
 
   minijail_free_env(env);
 }
+
+TEST(parse_single_constant, formats) {
+  char *end;
+  long int c = 0;
+  std::string constant;
+
+  // Check base 10 works.
+  constant = "1234";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(1234, c);
+
+  // Check base 16 works.
+  constant = "0x1234";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(0x1234, c);
+
+  // Check base 8 works.
+  constant = "01234";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(01234, c);
+}
+
+TEST(parse_constant, unsigned) {
+  char *end;
+  long int c = 0;
+  std::string constant;
+
+#if defined(BITS32)
+  constant = "0x80000000";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(0x80000000U, static_cast<unsigned long int>(c));
+
+#elif defined(BITS64)
+  constant = "0x8000000000000000";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(0x8000000000000000UL, static_cast<unsigned long int>(c));
+
+#else
+# error "unknown bits!"
+#endif
+}
+
+TEST(parse_constant, unsigned_toobig) {
+  char *end;
+  long int c = 0;
+  std::string constant;
+
+#if defined(BITS32)
+  constant = "0x100000000";  // Too big for 32-bit unsigned long int.
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  // Error case should return 0.
+  EXPECT_EQ(0, c);
+
+#elif defined(BITS64)
+  constant = "0x10000000000000000";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  // Error case should return 0.
+  EXPECT_EQ(0, c);
+
+#else
+# error "unknown bits!"
+#endif
+}
+
+TEST(parse_constant, signed) {
+  char *end;
+  long int c = 0;
+  std::string constant = "-1";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(-1, c);
+}
+
+TEST(parse_constant, signed_toonegative) {
+  char *end;
+  long int c = 0;
+  std::string constant;
+
+#if defined(BITS32)
+  constant = "-0x80000001";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  // Error case should return 0.
+  EXPECT_EQ(0, c);
+
+#elif defined(BITS64)
+  constant = "-0x8000000000000001";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  // Error case should return 0.
+  EXPECT_EQ(0, c);
+
+#else
+# error "unknown bits!"
+#endif
+}
+
+TEST(parse_constant, complements) {
+  char* end;
+  long int c = 0;
+  std::string constant;
+
+#if defined(BITS32)
+  constant = "~0x005AF0FF|~0xFFA50FFF";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(c, 0xFFFFFF00);
+  constant = "0x0F|~(0x005AF000|0x00A50FFF)|0xF0";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(c, 0xFF0000FF);
+
+#elif defined(BITS64)
+  constant = "~0x00005A5AF0F0FFFF|~0xFFFFA5A50F0FFFFF";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(c, 0xFFFFFFFFFFFF0000UL);
+  constant = "0x00FF|~(0x00005A5AF0F00000|0x0000A5A50F0FFFFF)|0xFF00";
+  c = parse_constant(const_cast<char*>(constant.data()), &end);
+  EXPECT_EQ(c, 0xFFFF00000000FFFFUL);
+
+#else
+# error "unknown bits!"
+#endif
+}
+
+TEST(parse_constant, parenthesized_expresions) {
+  char* end;
+
+  const std::vector<const char*> bad_expressions = {
+      "(1", "1)", "(1)1", "|(1)", "(1)|", "()",
+      "(",  "((", "(()",  "(()1", "1(0)",
+  };
+  for (const auto* expression : bad_expressions) {
+    std::string mutable_expression = expression;
+    long int c =
+        parse_constant(const_cast<char*>(mutable_expression.data()), &end);
+    EXPECT_EQ(reinterpret_cast<const void*>(end),
+              reinterpret_cast<const void*>(mutable_expression.data()));
+    // Error case should return 0.
+    EXPECT_EQ(c, 0) << "For expression: \"" << expression << "\"";
+  }
+
+  const std::vector<const char*> good_expressions = {
+      "(3)", "(1)|2", "1|(2)", "(1)|(2)", "((3))", "0|(1|2)", "(0|1|2)",
+  };
+  for (const auto* expression : good_expressions) {
+    std::string mutable_expression = expression;
+    long int c =
+        parse_constant(const_cast<char*>(mutable_expression.data()), &end);
+    EXPECT_EQ(c, 3) << "For expression: \"" << expression << "\"";
+  }
+}
+
+TEST(parse_size, complete) {
+  size_t size;
+
+  ASSERT_EQ(0, parse_size(&size, "42"));
+  ASSERT_EQ(42U, size);
+
+  ASSERT_EQ(0, parse_size(&size, "16K"));
+  ASSERT_EQ(16384U, size);
+
+  ASSERT_EQ(0, parse_size(&size, "1M"));
+  ASSERT_EQ(1024U * 1024, size);
+
+  uint64_t gigabyte = 1024ULL * 1024 * 1024;
+  ASSERT_EQ(0, parse_size(&size, "3G"));
+  ASSERT_EQ(3U, size / gigabyte);
+  ASSERT_EQ(0U, size % gigabyte);
+
+  ASSERT_EQ(0, parse_size(&size, "4294967294"));
+  ASSERT_EQ(3U, size / gigabyte);
+  ASSERT_EQ(gigabyte - 2, size % gigabyte);
+
+#if __WORDSIZE == 64
+  uint64_t exabyte = gigabyte * 1024 * 1024 * 1024;
+  ASSERT_EQ(0, parse_size(&size, "9E"));
+  ASSERT_EQ(9U, size / exabyte);
+  ASSERT_EQ(0U, size % exabyte);
+
+  ASSERT_EQ(0, parse_size(&size, "15E"));
+  ASSERT_EQ(15U, size / exabyte);
+  ASSERT_EQ(0U, size % exabyte);
+
+  ASSERT_EQ(0, parse_size(&size, "18446744073709551614"));
+  ASSERT_EQ(15U, size / exabyte);
+  ASSERT_EQ(exabyte - 2, size % exabyte);
+
+  ASSERT_EQ(-ERANGE, parse_size(&size, "16E"));
+  ASSERT_EQ(-ERANGE, parse_size(&size, "19E"));
+  ASSERT_EQ(-EINVAL, parse_size(&size, "7GTPE"));
+#elif __WORDSIZE == 32
+  ASSERT_EQ(-ERANGE, parse_size(&size, "5G"));
+  ASSERT_EQ(-ERANGE, parse_size(&size, "9G"));
+  ASSERT_EQ(-ERANGE, parse_size(&size, "9E"));
+  ASSERT_EQ(-ERANGE, parse_size(&size, "7GTPE"));
+#endif
+
+  ASSERT_EQ(-EINVAL, parse_size(&size, ""));
+  ASSERT_EQ(-EINVAL, parse_size(&size, "14u"));
+  ASSERT_EQ(-EINVAL, parse_size(&size, "14.2G"));
+  ASSERT_EQ(-EINVAL, parse_size(&size, "-1G"));
+  ASSERT_EQ(-EINVAL, parse_size(&size, "; /bin/rm -- "));
+}