Re-add seccomp policy tracking.
This makes it easier to attribute a policy violation to the policy and
makes it possible for crash reporter to grab the environment
variable and include it in the crash report.
This reverts:
https://android-review.googlesource.com/c/platform/external/minijail/+/1839493
Relanding:
https://android-review.googlesource.com/c/platform/external/minijail/+/1824233
https://android-review.googlesource.com/c/platform/external/minijail/+/1824238
Bug: 199444797
Test: initctl restart ui # and check /var/log/messages for arc++ crash
Test: tast run <host> crash.Seccomp
Change-Id: I1cfdc30a734ff98bf8777ebc8997011b922f0b3c
diff --git a/libminijail.c b/libminijail.c
index 13df237..265c5f2 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -189,6 +189,7 @@
struct hook *hooks_tail;
struct preserved_fd preserved_fds[MAX_PRESERVED_FDS];
size_t preserved_fd_count;
+ char *seccomp_policy_path;
};
static void run_hooks_or_die(const struct minijail *j,
@@ -984,6 +985,10 @@
j->filter_len = 0;
j->filter_prog = NULL;
j->flags.no_new_privs = 0;
+ if (j->seccomp_policy_path) {
+ free(j->seccomp_policy_path);
+ }
+ j->seccomp_policy_path = NULL;
}
static int seccomp_should_use_filters(struct minijail *j)
@@ -1145,6 +1150,10 @@
die("failed to compile seccomp filter BPF program in '%s'",
path);
}
+ if (j->seccomp_policy_path) {
+ free(j->seccomp_policy_path);
+ }
+ j->seccomp_policy_path = strdup(path);
}
void API minijail_parse_seccomp_filters_from_fd(struct minijail *j, int fd)
@@ -1171,7 +1180,10 @@
die("failed to compile seccomp filter BPF program from fd %d",
fd);
}
- free(path);
+ if (j->seccomp_policy_path) {
+ free(j->seccomp_policy_path);
+ }
+ j->seccomp_policy_path = path;
}
void API minijail_set_seccomp_filters(struct minijail *j,
@@ -1281,6 +1293,8 @@
}
for (i = 0; i < j->cgroup_count; ++i)
marshal_append_string(state, j->cgroups[i]);
+ if (j->seccomp_policy_path)
+ marshal_append_string(state, j->seccomp_policy_path);
}
size_t API minijail_size(const struct minijail *j)
@@ -1445,8 +1459,23 @@
++j->cgroup_count;
}
+ if (j->seccomp_policy_path) { /* stale pointer */
+ char *seccomp_policy_path = consumestr(&serialized, &length);
+ if (!seccomp_policy_path)
+ goto bad_cgroups;
+ j->seccomp_policy_path = strdup(seccomp_policy_path);
+ if (!j->seccomp_policy_path)
+ goto bad_cgroups;
+ }
+
return 0;
+ /*
+ * If more is added after j->seccomp_policy_path, then this is needed:
+ * if (j->seccomp_policy_path)
+ * free(j->seccomp_policy_path);
+ */
+
bad_cgroups:
free_mounts_list(j);
free_remounts_list(j);
@@ -1480,6 +1509,7 @@
j->hostname = NULL;
j->alt_syscall_table = NULL;
j->cgroup_count = 0;
+ j->seccomp_policy_path = NULL;
out:
return ret;
}
@@ -2587,6 +2617,19 @@
#endif
}
+/*
+ * This is for logging purposes and does not change the enforced seccomp
+ * filter.
+ */
+static int setup_seccomp_policy_path(const struct minijail *j,
+ char ***child_env)
+{
+ return minijail_setenv(child_env, kSeccompPolicyPathEnvVar,
+ j->seccomp_policy_path ? j->seccomp_policy_path
+ : "NO-LABEL",
+ 1 /* overwrite */);
+}
+
static int setup_pipe(char ***child_env, int fds[2])
{
int r = pipe(fds);
@@ -3079,6 +3122,21 @@
die("filename and elf_fd cannot be set at the same time");
}
+ /*
+ * Only copy the environment if we need to modify it. If this is done
+ * unconditionally, it triggers odd behavior in the ARC container.
+ */
+ if (use_preload || j->seccomp_policy_path) {
+ state_out->child_env =
+ minijail_copy_env(config->envp ? config->envp : environ);
+ if (!state_out->child_env)
+ return ENOMEM;
+ }
+
+ if (j->seccomp_policy_path &&
+ setup_seccomp_policy_path(j, &state_out->child_env))
+ return -EFAULT;
+
if (use_preload) {
if (j->hooks_head != NULL)
die("Minijail hooks are not supported with LD_PRELOAD");
@@ -3089,10 +3147,6 @@
* 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;
@@ -3527,24 +3581,30 @@
if (!WIFEXITED(st)) {
int error_status = st;
- if (WIFSIGNALED(st)) {
- int signum = WTERMSIG(st);
+ if (!WIFSIGNALED(st)) {
+ return error_status;
+ }
+
+ int signum = WTERMSIG(st);
+ /*
+ * We return MINIJAIL_ERR_JAIL if the process received
+ * SIGSYS, which happens when a syscall is blocked by
+ * seccomp filters.
+ * If not, we do what bash(1) does:
+ * $? = 128 + signum
+ */
+ if (signum == SIGSYS) {
+ warn("child process %d had a policy violation (%s)",
+ j->initpid,
+ j->seccomp_policy_path ? j->seccomp_policy_path
+ : "NO-LABEL");
+ error_status = MINIJAIL_ERR_JAIL;
+ } else {
if (signum != expected_signal) {
warn("child process %d received signal %d",
j->initpid, signum);
}
- /*
- * We return MINIJAIL_ERR_JAIL if the process received
- * SIGSYS, which happens when a syscall is blocked by
- * seccomp filters.
- * If not, we do what bash(1) does:
- * $? = 128 + signum
- */
- if (signum == SIGSYS) {
- error_status = MINIJAIL_ERR_JAIL;
- } else {
- error_status = MINIJAIL_ERR_SIG_BASE + signum;
- }
+ error_status = MINIJAIL_ERR_SIG_BASE + signum;
}
return error_status;
}
@@ -3609,6 +3669,8 @@
free(j->alt_syscall_table);
for (i = 0; i < j->cgroup_count; ++i)
free(j->cgroups[i]);
+ if (j->seccomp_policy_path)
+ free(j->seccomp_policy_path);
free(j);
}
