Snap for 4557233 from dc31c08fbb28e71b35c17170b82baace8dde53a1 to pi-release
Change-Id: If398ca1c31d2636a2b2749a48102d8284eff902a
diff --git a/.gitignore b/.gitignore
index a121c9e..8713a1e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,6 +12,7 @@
/minijail0
/syscall_filter_unittest
/system_unittest
+/util_unittest
/parse_seccomp_policy
# common.mk TEST().
diff --git a/Android.bp b/Android.bp
index 03b0530..9dd17ea 100644
--- a/Android.bp
+++ b/Android.bp
@@ -236,6 +236,34 @@
},
}
+// Utility functionality unit tests using gtest.
+//
+// For a device, run with:
+// adb shell /data/nativetest/util_unittest_gtest/util_unittest_gtest
+//
+// For host, run with:
+// out/host/linux-x86/nativetest(64)/util_unittest_gtest/util_unittest_gtest
+// =========================================================
+cc_test {
+ name: "util_unittest_gtest",
+ defaults: ["libminijail_flags"],
+ host_supported: true,
+
+ srcs: [
+ "util.c",
+ "util_unittest.cc",
+ ] + unittestSrcFiles,
+
+ static_libs: ["libminijail_generated"],
+ shared_libs: minijailCommonLibraries,
+
+ target: {
+ android: {
+ test_suites: ["device-tests"],
+ },
+ },
+}
+
// libminijail_test executable for brillo_Minijail test.
// =========================================================
cc_test {
diff --git a/Makefile b/Makefile
index b0f0509..7bbb0b4 100644
--- a/Makefile
+++ b/Makefile
@@ -55,7 +55,8 @@
tests: TEST(CXX_BINARY(libminijail_unittest)) \
TEST(CXX_BINARY(syscall_filter_unittest)) \
- TEST(CXX_BINARY(system_unittest))
+ TEST(CXX_BINARY(system_unittest)) \
+ TEST(CXX_BINARY(util_unittest)) \
CC_BINARY(minijail0): LDLIBS += -lcap -ldl
@@ -105,6 +106,16 @@
clean: CLEAN(system_unittest)
+CXX_BINARY(util_unittest): CXXFLAGS += $(GTEST_CXXFLAGS)
+CXX_BINARY(util_unittest): LDLIBS += -lcap $(GTEST_LIBS)
+ifeq ($(USE_SYSTEM_GTEST),no)
+CXX_BINARY(util_unittest): $(GTEST_LIBS)
+endif
+CXX_BINARY(util_unittest): util_unittest.o \
+ $(CORE_OBJECT_FILES) testrunner.o
+clean: CLEAN(util_unittest)
+
+
CXX_BINARY(parse_seccomp_policy): parse_seccomp_policy.o syscall_filter.o \
bpf.o util.o libconstants.gen.o libsyscalls.gen.o
clean: CLEAN(parse_seccomp_policy)
diff --git a/gen_constants-inl.h b/gen_constants-inl.h
index ea8c701..8efd4f6 100644
--- a/gen_constants-inl.h
+++ b/gen_constants-inl.h
@@ -10,6 +10,7 @@
#include <stddef.h>
#include <signal.h>
#include <sys/mman.h>
-#include <sys/stat.h>
#include <sys/resource.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
#include <sys/types.h>
diff --git a/minijail0.1 b/minijail0.1
index 9d24422..52f98e2 100644
--- a/minijail0.1
+++ b/minijail0.1
@@ -12,9 +12,10 @@
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.
.TP
@@ -58,13 +59,14 @@
\fB-H\fR
Print a help message detailing supported system call names for seccomp_filter.
(Other direct numbers may be specified if minijail0 is not in sync with the
- host kernel or something like 32/64-bit compatibility issues exist.)
+host kernel or something like 32/64-bit compatibility issues exist.)
.TP
\fB-I\fR
Run \fIprogram\fR as init (pid 1) inside a new pid namespace (implies \fB-p\fR).
.TP
-\fB-k <src>,<dest>,<type>[,<flags>]\fR
-Mount \fIsrc\fR, a \fItype\fR filesystem, into the chroot directory at \fIdest\fR, with optional \fIflags\fR.
+\fB-k <src>,<dest>,<type>[,<flags>[,<data>]]\fR
+Mount \fIsrc\fR, a \fItype\fR filesystem, into the chroot directory at \fIdest\fR,
+with optional \fIflags\fR and optional \fIdata\fR (see \fBmount\fR(2)).
If the mount is not a pseudo filesystem (e.g. proc or sysfs), \fIsrc\fR path
must be an absolute path (e.g. \fI/dev/sda1\fR and not \fIsda1\fR).
If the destination does not exist, it will be created as a directory.
diff --git a/minijail0.c b/minijail0.c
index b0daed2..425fbf0 100644
--- a/minijail0.c
+++ b/minijail0.c
@@ -92,14 +92,18 @@
static void add_binding(struct minijail *j, char *arg)
{
- char *src = strtok(arg, ",");
- char *dest = strtok(NULL, ",");
- char *flags = strtok(NULL, ",");
- if (!src || !dest) {
+ char *src = tokenize(&arg, ",");
+ char *dest = tokenize(&arg, ",");
+ char *flags = tokenize(&arg, ",");
+ if (!src || src[0] == '\0' || arg != NULL) {
fprintf(stderr, "Bad binding: %s %s\n", src, dest);
exit(1);
}
- if (minijail_bind(j, src, dest, flags ? atoi(flags) : 0)) {
+ if (dest == NULL || dest[0] == '\0')
+ dest = src;
+ if (flags == NULL || flags[0] == '\0')
+ flags = "0";
+ if (minijail_bind(j, src, dest, atoi(flags))) {
fprintf(stderr, "minijail_bind failed.\n");
exit(1);
}
@@ -107,10 +111,11 @@
static void add_rlimit(struct minijail *j, char *arg)
{
- char *type = strtok(arg, ",");
- char *cur = strtok(NULL, ",");
- char *max = strtok(NULL, ",");
- if (!type || !cur || !max) {
+ char *type = tokenize(&arg, ",");
+ char *cur = tokenize(&arg, ",");
+ char *max = tokenize(&arg, ",");
+ if (!type || type[0] == '\0' || !cur || cur[0] == '\0' ||
+ !max || max[0] == '\0' || arg != NULL) {
fprintf(stderr, "Bad rlimit '%s'.\n", arg);
exit(1);
}
@@ -123,12 +128,13 @@
static void add_mount(struct minijail *j, char *arg)
{
- char *src = strtok(arg, ",");
- char *dest = strtok(NULL, ",");
- char *type = strtok(NULL, ",");
- char *flags = strtok(NULL, ",");
- char *data = strtok(NULL, ",");
- if (!src || !dest || !type) {
+ char *src = tokenize(&arg, ",");
+ char *dest = tokenize(&arg, ",");
+ char *type = tokenize(&arg, ",");
+ char *flags = tokenize(&arg, ",");
+ char *data = tokenize(&arg, ",");
+ if (!src || src[0] == '\0' || !dest || dest[0] == '\0' ||
+ !type || type[0] == '\0' || arg != NULL) {
fprintf(stderr, "Bad mount: %s %s %s\n", src, dest, type);
exit(1);
}
@@ -294,7 +300,7 @@
/* 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"
diff --git a/syscall_filter_unittest.cc b/syscall_filter_unittest.cc
index 6bc044d..db01fbb 100644
--- a/syscall_filter_unittest.cc
+++ b/syscall_filter_unittest.cc
@@ -1261,6 +1261,19 @@
ASSERT_NE(res, 0);
}
+TEST(FilterTest, invalid_tokens) {
+ struct sock_fprog actual;
+ const char *policy = "read: arg0 == 1 |||| arg0 == 2\n";
+
+ FILE *policy_file = write_policy_to_pipe(policy, strlen(policy));
+ ASSERT_NE(policy_file, nullptr);
+
+ int res =
+ compile_filter("policy", policy_file, &actual, USE_RET_KILL, NO_LOGGING);
+ fclose(policy_file);
+ ASSERT_NE(res, 0);
+}
+
TEST(FilterTest, nonexistent) {
struct sock_fprog actual;
int res = compile_filter("policy", NULL, &actual, USE_RET_KILL, NO_LOGGING);
diff --git a/util.c b/util.c
index 14c028a..9bb37ca 100644
--- a/util.c
+++ b/util.c
@@ -257,8 +257,8 @@
{
char *ret = NULL;
- /* If the string is NULL or empty, there are no tokens to be found. */
- if (stringp == NULL || *stringp == NULL || **stringp == '\0')
+ /* If the string is NULL, there are no tokens to be found. */
+ if (stringp == NULL || *stringp == NULL)
return NULL;
/*
@@ -271,33 +271,19 @@
return ret;
}
- char *found;
- while (**stringp != '\0') {
- found = strstr(*stringp, delim);
-
- if (!found) {
- /*
- * The delimiter was not found, so the full string
- * makes up the only token, and we're done.
- */
- ret = *stringp;
- *stringp = NULL;
- break;
- }
-
- if (found != *stringp) {
- /* There's a non-empty token before the delimiter. */
- *found = '\0';
- ret = *stringp;
- *stringp = found + strlen(delim);
- break;
- }
-
+ char *found = strstr(*stringp, delim);
+ if (!found) {
/*
- * The delimiter was found at the start of the string,
- * skip it and keep looking for a non-empty token.
+ * The delimiter was not found, so the full string
+ * makes up the only token, and we're done.
*/
- *stringp += strlen(delim);
+ ret = *stringp;
+ *stringp = NULL;
+ } else {
+ /* There's a token here, possibly empty. That's OK. */
+ *found = '\0';
+ ret = *stringp;
+ *stringp = found + strlen(delim);
}
return ret;
diff --git a/util.h b/util.h
index 9ec88ce..7ff86b8 100644
--- a/util.h
+++ b/util.h
@@ -83,6 +83,18 @@
int parse_size(size_t *size, const char *sizespec);
char *strip(char *s);
+
+/*
+ * tokenize: locate the next token in @stringp using the @delim
+ * @stringp A pointer to the string to scan for tokens
+ * @delim The delimiter to split by
+ *
+ * Note that, unlike strtok, @delim is not a set of characters, but the full
+ * delimiter. e.g. "a,;b,;c" with a delim of ",;" will yield ["a","b","c"].
+ *
+ * Note that, unlike strtok, this may return an empty token. e.g. "a,,b" with
+ * strtok will yield ["a","b"], but this will yield ["a","","b"].
+ */
char *tokenize(char **stringp, const char *delim);
char *path_join(const char *external_path, const char *internal_path);
diff --git a/util_unittest.cc b/util_unittest.cc
new file mode 100644
index 0000000..ec3d714
--- /dev/null
+++ b/util_unittest.cc
@@ -0,0 +1,93 @@
+// util_unittest.cpp
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Test system.[ch] module code using gtest.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <gtest/gtest.h>
+
+#include "util.h"
+
+// Sanity check for the strip func.
+TEST(strip, basic) {
+ char str[] = " foo\t";
+ ASSERT_EQ("foo", std::string(strip(str)));
+}
+
+// Make sure we don't crash with various "null"-like inputs.
+TEST(tokenize, null_stringp) {
+ ASSERT_EQ(nullptr, tokenize(nullptr, nullptr));
+ ASSERT_EQ(nullptr, tokenize(nullptr, ""));
+ ASSERT_EQ(nullptr, tokenize(nullptr, ","));
+
+ char *p = nullptr;
+ ASSERT_EQ(nullptr, tokenize(&p, nullptr));
+}
+
+// Make sure we don't crash with various "null"-like inputs.
+TEST(tokenize, null_delim) {
+ char str[] = "a,b,c";
+ char *p = str;
+ ASSERT_EQ(str, tokenize(&p, nullptr));
+ ASSERT_EQ(nullptr, p);
+ ASSERT_EQ(str, std::string("a,b,c"));
+
+ p = str;
+ ASSERT_EQ(str, tokenize(&p, ""));
+ ASSERT_EQ(nullptr, p);
+ ASSERT_EQ(str, std::string("a,b,c"));
+}
+
+// Sanity check for the tokenize func.
+TEST(tokenize, basic) {
+ char str[] = "a,b,c";
+ char *p = str;
+ ASSERT_EQ("a", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("b", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("c", std::string(tokenize(&p, ",")));
+ ASSERT_EQ(nullptr, p);
+ ASSERT_EQ(nullptr, tokenize(&p, ","));
+}
+
+// Check edge case with an empty string.
+TEST(tokenize, empty_string) {
+ char str[] = "";
+ char *p = str;
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ(nullptr, p);
+ ASSERT_EQ(nullptr, tokenize(&p, ","));
+}
+
+// Check behavior with empty tokens at the start/middle/end.
+TEST(tokenize, empty_tokens) {
+ char str[] = ",,a,b,,,c,,";
+ char *p = str;
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("a", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("b", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("c", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ("", std::string(tokenize(&p, ",")));
+ ASSERT_EQ(nullptr, p);
+ ASSERT_EQ(nullptr, tokenize(&p, ","));
+}