linux-mips
[Top] [All Lists]

[PATCH v6 2/9] seccomp: split filter prep from check and apply

To: linux-kernel@vger.kernel.org
Subject: [PATCH v6 2/9] seccomp: split filter prep from check and apply
From: Kees Cook <keescook@chromium.org>
Date: Tue, 10 Jun 2014 16:01:47 -0700
Cc: Kees Cook <keescook@chromium.org>, Andy Lutomirski <luto@amacapital.net>, Oleg Nesterov <oleg@redhat.com>, Will Drewry <wad@chromium.org>, Julien Tinnes <jln@chromium.org>, David Drysdale <drysdale@google.com>, Alexei Starovoitov <ast@plumgrid.com>, John Johansen <john.johansen@canonical.com>, Russell King <linux@arm.linux.org.uk>, Ralf Baechle <ralf@linux-mips.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, Alexander Viro <viro@zeniv.linux.org.uk>, Peter Zijlstra <peterz@infradead.org>, Arnd Bergmann <arnd@arndb.de>, James Morris <james.l.morris@oracle.com>, Andrew Morton <akpm@linux-foundation.org>, Frederic Weisbecker <fweisbec@gmail.com>, "David A. Long" <dave.long@linaro.org>, Heiko Carstens <heiko.carstens@de.ibm.com>, Kevin Hilman <khilman@linaro.org>, Christoph Hellwig <hch@infradead.org>, Michal Simek <michal.simek@xilinx.com>, Juri Lelli <juri.lelli@gmail.com>, Miklos Szeredi <mszeredi@suse.cz>, Dario Faggioli <raistlin@linux.it>, Markos Chandras <markos.chandras@imgtec.com>, James Hogan <james.hogan@imgtec.com>, Huacai Chen <chenhc@lemote.com>, Paul Burton <paul.burton@imgtec.com>, Viresh Kumar <viresh.kumar@linaro.org>, "J. Bruce Fields" <bfields@redhat.com>, Mike Frysinger <vapier@gentoo.org>, "David S. Miller" <davem@davemloft.net>, Daniel Borkmann <dborkman@redhat.com>, Steven Rostedt <rostedt@goodmis.org>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, Tom Zanussi <tom.zanussi@linux.intel.com>, Andi Kleen <ak@linux.intel.com>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Rik van Riel <riel@redhat.com>, Daeseok Youn <daeseok.youn@gmail.com>, David Rientjes <rientjes@google.com>, Eric Paris <eparis@redhat.com>, Fabian Frederick <fabf@skynet.be>, Robin Holt <holt@sgi.com>, Dongsheng Yang <yangds.fnst@cn.fujitsu.com>, liguang <lig.fnst@cn.fujitsu.com>, Geert Uytterhoeven <geert@linux-m68k.org>, Alex Thorlton <athorlton@sgi.com>, "Eric W. Biederman" <ebiederm@xmission.com>, Josh Triplett <josh@joshtriplett.org>, linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-security-module@vger.kernel.org, x86@kernel.org
In-reply-to: <1402441314-7447-1-git-send-email-keescook@chromium.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1402441314-7447-1-git-send-email-keescook@chromium.org>
Sender: linux-mips-bounce@linux-mips.org
In preparation for adding seccomp locking, move filter creation away
from where it is checked and applied. This will allow for locking where
no memory allocation is happening. The validation, filter attachment,
and seccomp mode setting can all happen under the future locks.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 kernel/seccomp.c |   86 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 28 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 552b972b8f83..7a9257ddd69c 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -18,6 +18,7 @@
 #include <linux/compat.h>
 #include <linux/sched.h>
 #include <linux/seccomp.h>
+#include <linux/slab.h>
 
 /* #define SECCOMP_DEBUG 1 */
 
@@ -26,7 +27,6 @@
 #include <linux/filter.h>
 #include <linux/ptrace.h>
 #include <linux/security.h>
-#include <linux/slab.h>
 #include <linux/tracehook.h>
 #include <linux/uaccess.h>
 
@@ -197,27 +197,21 @@ static u32 seccomp_run_filters(int syscall)
 }
 
 /**
- * seccomp_attach_filter: Attaches a seccomp filter to current.
+ * seccomp_prepare_filter: Prepares a seccomp filter for use.
  * @fprog: BPF program to install
  *
- * Returns 0 on success or an errno on failure.
+ * Returns filter on success or an ERR_PTR on failure.
  */
-static long seccomp_attach_filter(struct sock_fprog *fprog)
+static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
 {
        struct seccomp_filter *filter;
        unsigned long fp_size = fprog->len * sizeof(struct sock_filter);
-       unsigned long total_insns = fprog->len;
        struct sock_filter *fp;
        int new_len;
        long ret;
 
        if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
-               return -EINVAL;
-
-       for (filter = current->seccomp.filter; filter; filter = filter->prev)
-               total_insns += filter->len + 4;  /* include a 4 instr penalty */
-       if (total_insns > MAX_INSNS_PER_PATH)
-               return -ENOMEM;
+               return ERR_PTR(-EINVAL);
 
        /*
         * Installing a seccomp filter requires that the task has
@@ -228,11 +222,11 @@ static long seccomp_attach_filter(struct sock_fprog 
*fprog)
        if (!current->no_new_privs &&
            security_capable_noaudit(current_cred(), current_user_ns(),
                                     CAP_SYS_ADMIN) != 0)
-               return -EACCES;
+               return ERR_PTR(-EACCES);
 
        fp = kzalloc(fp_size, GFP_KERNEL|__GFP_NOWARN);
        if (!fp)
-               return -ENOMEM;
+               return ERR_PTR(-ENOMEM);
 
        /* Copy the instructions from fprog. */
        ret = -EFAULT;
@@ -270,31 +264,26 @@ static long seccomp_attach_filter(struct sock_fprog 
*fprog)
        atomic_set(&filter->usage, 1);
        filter->len = new_len;
 
-       /*
-        * If there is an existing filter, make it the prev and don't drop its
-        * task reference.
-        */
-       filter->prev = current->seccomp.filter;
-       current->seccomp.filter = filter;
-       return 0;
+       return filter;
 
 free_filter:
        kfree(filter);
 free_prog:
        kfree(fp);
-       return ret;
+       return ERR_PTR(ret);
 }
 
 /**
- * seccomp_attach_user_filter - attaches a user-supplied sock_fprog
+ * seccomp_prepare_user_filter - prepares a user-supplied sock_fprog
  * @user_filter: pointer to the user data containing a sock_fprog.
  *
- * Returns 0 on success and non-zero otherwise.
+ * Returns filter on success and ERR_PTR otherwise.
  */
-static long seccomp_attach_user_filter(char __user *user_filter)
+static
+struct seccomp_filter *seccomp_prepare_user_filter(char __user *user_filter)
 {
        struct sock_fprog fprog;
-       long ret = -EFAULT;
+       struct seccomp_filter *filter = ERR_PTR(-EFAULT);
 
 #ifdef CONFIG_COMPAT
        if (is_compat_task()) {
@@ -307,9 +296,37 @@ static long seccomp_attach_user_filter(char __user 
*user_filter)
 #endif
        if (copy_from_user(&fprog, user_filter, sizeof(fprog)))
                goto out;
-       ret = seccomp_attach_filter(&fprog);
+       filter = seccomp_prepare_filter(&fprog);
 out:
-       return ret;
+       return filter;
+}
+
+/**
+ * seccomp_attach_filter: validate and attach filter
+ * @filter: seccomp filter to add to the current process
+ *
+ * Returns 0 on success, -ve on error.
+ */
+static long seccomp_attach_filter(struct seccomp_filter *filter)
+{
+       unsigned long total_insns;
+       struct seccomp_filter *walker;
+
+       /* Validate resulting filter length. */
+       total_insns = filter->len;
+       for (walker = current->seccomp.filter; walker; walker = filter->prev)
+               total_insns += walker->len + 4;  /* include a 4 instr penalty */
+       if (total_insns > MAX_INSNS_PER_PATH)
+               return -ENOMEM;
+
+       /*
+        * If there is an existing filter, make it the prev and don't drop its
+        * task reference.
+        */
+       filter->prev = current->seccomp.filter;
+       current->seccomp.filter = filter;
+
+       return 0;
 }
 
 /* get_seccomp_filter - increments the reference count of the filter on @tsk */
@@ -480,8 +497,18 @@ long prctl_get_seccomp(void)
  */
 static long seccomp_set_mode(unsigned long seccomp_mode, char __user *filter)
 {
+       struct seccomp_filter *prepared = NULL;
        long ret = -EINVAL;
 
+#ifdef CONFIG_SECCOMP_FILTER
+       /* Prepare the new filter outside of the seccomp lock. */
+       if (seccomp_mode == SECCOMP_MODE_FILTER) {
+               prepared = seccomp_prepare_user_filter(filter);
+               if (IS_ERR(prepared))
+                       return PTR_ERR(prepared);
+       }
+#endif
+
        if (current->seccomp.mode &&
            current->seccomp.mode != seccomp_mode)
                goto out;
@@ -495,9 +522,11 @@ static long seccomp_set_mode(unsigned long seccomp_mode, 
char __user *filter)
                break;
 #ifdef CONFIG_SECCOMP_FILTER
        case SECCOMP_MODE_FILTER:
-               ret = seccomp_attach_user_filter(filter);
+               ret = seccomp_attach_filter(prepared);
                if (ret)
                        goto out;
+               /* Do not free the successfully attached filter. */
+               prepared = NULL;
                break;
 #endif
        default:
@@ -507,6 +536,7 @@ static long seccomp_set_mode(unsigned long seccomp_mode, 
char __user *filter)
        current->seccomp.mode = seccomp_mode;
        set_thread_flag(TIF_SECCOMP);
 out:
+       kfree(prepared);
        return ret;
 }
 
-- 
1.7.9.5


<Prev in Thread] Current Thread [Next in Thread>