Clarify comments around minijail_pre{enter|exec}().
I was investigating Minijail behaviour with static binaries and
it took me a while to figure out what was happening where.
Document preenter/preexec functions better and move them closer
to the flags they track. This way if we add a new flag in the future
we'll also track it in minijail_pre{enter|exec}().
BUG=None
TEST=unit, security_{Minijail0|Minijail_seccomp} on leon.
Change-Id: I67e1e233b0fa0df2dcd97ad397187a7dc791a0c3
Reviewed-on: https://chromium-review.googlesource.com/194200
Reviewed-by: Kees Cook <keescook@chromium.org>
Commit-Queue: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
diff --git a/libminijail.c b/libminijail.c
index 8a1300d..c764f81 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -66,6 +66,10 @@
};
struct minijail {
+ /*
+ * WARNING: if you add a flag here you need to make sure it's accounted for
+ * in minijail_pre{enter|exec}() below.
+ */
struct {
int uid:1;
int gid:1;
@@ -96,6 +100,38 @@
struct binding *bindings_tail;
};
+/*
+ * Strip out flags meant for the parent.
+ * We keep things that are not inherited across execve(2) (e.g. capabilities),
+ * or are easier to set after execve(2) (e.g. seccomp filters).
+ */
+void minijail_preenter(struct minijail *j)
+{
+ j->flags.vfs = 0;
+ j->flags.readonly = 0;
+ j->flags.pids = 0;
+}
+
+/*
+ * Strip out flags meant for the child.
+ * We keep things that are inherited across execve(2).
+ */
+void minijail_preexec(struct minijail *j)
+{
+ int vfs = j->flags.vfs;
+ int readonly = j->flags.readonly;
+ if (j->user)
+ free(j->user);
+ j->user = NULL;
+ memset(&j->flags, 0, sizeof(j->flags));
+ /* Now restore anything we meant to keep. */
+ j->flags.vfs = vfs;
+ j->flags.readonly = readonly;
+ /* Note, |pids| will already have been used before this call. */
+}
+
+/* Minijail API. */
+
struct minijail API *minijail_new(void)
{
return calloc(1, sizeof(struct minijail));
@@ -497,28 +533,6 @@
return ret;
}
-void minijail_preenter(struct minijail *j)
-{
- /* Strip out options which are minijail_run() only. */
- j->flags.vfs = 0;
- j->flags.readonly = 0;
- j->flags.pids = 0;
-}
-
-void minijail_preexec(struct minijail *j)
-{
- int vfs = j->flags.vfs;
- int readonly = j->flags.readonly;
- if (j->user)
- free(j->user);
- j->user = NULL;
- memset(&j->flags, 0, sizeof(j->flags));
- /* Now restore anything we meant to keep. */
- j->flags.vfs = vfs;
- j->flags.readonly = readonly;
- /* Note, pidns will already have been used before this call. */
-}
-
/* bind_one: Applies bindings from @b for @j, recursing as needed.
* @j Minijail these bindings are for
* @b Head of list of bindings
@@ -944,8 +958,8 @@
}
int API minijail_run_pid_pipes(struct minijail *j, const char *filename,
- char *const argv[], pid_t *pchild_pid,
- int *pstdin_fd, int *pstdout_fd, int *pstderr_fd)
+ char *const argv[], pid_t *pchild_pid,
+ int *pstdin_fd, int *pstdout_fd, int *pstderr_fd)
{
char *oldenv, *oldenv_copy = NULL;
pid_t child_pid;
@@ -1133,7 +1147,7 @@
die("failed to set up stderr pipe");
}
- /* Drop everything that cannot be inherited across execve. */
+ /* Strip out flags that cannot be inherited across execve. */
minijail_preexec(j);
/* Jail this process and its descendants... */
minijail_enter(j);
