Snap for 6844436 from 5f9e3001c61626d2863dad91248ba8496c3ef511 to simpleperf-releasesimpleperf-release

Change-Id: Iaab00c120bceec0b9617ba13f8b880d5a9d012ff

6 files changed, 273 insertions, 201 deletions

diff --git a/libminijail.c b/libminijail.c
index b1ec28bc..abe5a78b 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -724,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;
@@ -1152,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);
 
@@ -1174,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);
@@ -1185,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);
@@ -1215,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)
@@ -2211,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");
 		}
 	}
 
@@ -2336,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/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_cli.c b/minijail0_cli.c
index 22da7fd7..94d8578b 100644
--- a/minijail0_cli.c
+++ b/minijail0_cli.c
@@ -527,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"
@@ -645,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;
@@ -746,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':
@@ -780,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':
@@ -990,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/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_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 -- "));
+}