Commit c8bee430 authored by Kees Cook's avatar Kees Cook

seccomp: split filter prep from check and apply

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.

For extreme defensiveness, I've added a BUG_ON check for the calculated
size of the buffer allocation in case BPF_MAXINSN ever changes, which
shouldn't ever happen. The compiler should actually optimize out this
check since the test above it makes it impossible.
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Reviewed-by: default avatarOleg Nesterov <oleg@redhat.com>
Reviewed-by: default avatarAndy Lutomirski <luto@amacapital.net>
parent 1d4457f9
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include <linux/compat.h> #include <linux/compat.h>
#include <linux/sched.h> #include <linux/sched.h>
#include <linux/seccomp.h> #include <linux/seccomp.h>
#include <linux/slab.h>
#include <linux/syscalls.h> #include <linux/syscalls.h>
/* #define SECCOMP_DEBUG 1 */ /* #define SECCOMP_DEBUG 1 */
...@@ -27,7 +28,6 @@ ...@@ -27,7 +28,6 @@
#include <linux/filter.h> #include <linux/filter.h>
#include <linux/ptrace.h> #include <linux/ptrace.h>
#include <linux/security.h> #include <linux/security.h>
#include <linux/slab.h>
#include <linux/tracehook.h> #include <linux/tracehook.h>
#include <linux/uaccess.h> #include <linux/uaccess.h>
...@@ -213,27 +213,23 @@ static inline void seccomp_assign_mode(unsigned long seccomp_mode) ...@@ -213,27 +213,23 @@ static inline void seccomp_assign_mode(unsigned long seccomp_mode)
#ifdef CONFIG_SECCOMP_FILTER #ifdef CONFIG_SECCOMP_FILTER
/** /**
* seccomp_attach_filter: Attaches a seccomp filter to current. * seccomp_prepare_filter: Prepares a seccomp filter for use.
* @fprog: BPF program to install * @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; struct seccomp_filter *filter;
unsigned long fp_size = fprog->len * sizeof(struct sock_filter); unsigned long fp_size;
unsigned long total_insns = fprog->len;
struct sock_filter *fp; struct sock_filter *fp;
int new_len; int new_len;
long ret; long ret;
if (fprog->len == 0 || fprog->len > BPF_MAXINSNS) if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
return -EINVAL; return ERR_PTR(-EINVAL);
BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
for (filter = current->seccomp.filter; filter; filter = filter->prev) fp_size = fprog->len * sizeof(struct sock_filter);
total_insns += filter->prog->len + 4; /* include a 4 instr penalty */
if (total_insns > MAX_INSNS_PER_PATH)
return -ENOMEM;
/* /*
* Installing a seccomp filter requires that the task has * Installing a seccomp filter requires that the task has
...@@ -244,11 +240,11 @@ static long seccomp_attach_filter(struct sock_fprog *fprog) ...@@ -244,11 +240,11 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
if (!task_no_new_privs(current) && if (!task_no_new_privs(current) &&
security_capable_noaudit(current_cred(), current_user_ns(), security_capable_noaudit(current_cred(), current_user_ns(),
CAP_SYS_ADMIN) != 0) CAP_SYS_ADMIN) != 0)
return -EACCES; return ERR_PTR(-EACCES);
fp = kzalloc(fp_size, GFP_KERNEL|__GFP_NOWARN); fp = kzalloc(fp_size, GFP_KERNEL|__GFP_NOWARN);
if (!fp) if (!fp)
return -ENOMEM; return ERR_PTR(-ENOMEM);
/* Copy the instructions from fprog. */ /* Copy the instructions from fprog. */
ret = -EFAULT; ret = -EFAULT;
...@@ -292,13 +288,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog) ...@@ -292,13 +288,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
sk_filter_select_runtime(filter->prog); sk_filter_select_runtime(filter->prog);
/* return filter;
* 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;
free_filter_prog: free_filter_prog:
kfree(filter->prog); kfree(filter->prog);
...@@ -306,19 +296,20 @@ static long seccomp_attach_filter(struct sock_fprog *fprog) ...@@ -306,19 +296,20 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
kfree(filter); kfree(filter);
free_prog: free_prog:
kfree(fp); 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. * @user_filter: pointer to the user data containing a sock_fprog.
* *
* Returns 0 on success and non-zero otherwise. * Returns 0 on success and non-zero otherwise.
*/ */
static long seccomp_attach_user_filter(const char __user *user_filter) static struct seccomp_filter *
seccomp_prepare_user_filter(const char __user *user_filter)
{ {
struct sock_fprog fprog; struct sock_fprog fprog;
long ret = -EFAULT; struct seccomp_filter *filter = ERR_PTR(-EFAULT);
#ifdef CONFIG_COMPAT #ifdef CONFIG_COMPAT
if (is_compat_task()) { if (is_compat_task()) {
...@@ -331,9 +322,39 @@ static long seccomp_attach_user_filter(const char __user *user_filter) ...@@ -331,9 +322,39 @@ static long seccomp_attach_user_filter(const char __user *user_filter)
#endif #endif
if (copy_from_user(&fprog, user_filter, sizeof(fprog))) if (copy_from_user(&fprog, user_filter, sizeof(fprog)))
goto out; goto out;
ret = seccomp_attach_filter(&fprog); filter = seccomp_prepare_filter(&fprog);
out: out:
return ret; return filter;
}
/**
* seccomp_attach_filter: validate and attach filter
* @flags: flags to change filter behavior
* @filter: seccomp filter to add to the current process
*
* Returns 0 on success, -ve on error.
*/
static long seccomp_attach_filter(unsigned int flags,
struct seccomp_filter *filter)
{
unsigned long total_insns;
struct seccomp_filter *walker;
/* Validate resulting filter length. */
total_insns = filter->prog->len;
for (walker = current->seccomp.filter; walker; walker = walker->prev)
total_insns += walker->prog->len + 4; /* 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 */ /* get_seccomp_filter - increments the reference count of the filter on @tsk */
...@@ -346,6 +367,14 @@ void get_seccomp_filter(struct task_struct *tsk) ...@@ -346,6 +367,14 @@ void get_seccomp_filter(struct task_struct *tsk)
atomic_inc(&orig->usage); atomic_inc(&orig->usage);
} }
static inline void seccomp_filter_free(struct seccomp_filter *filter)
{
if (filter) {
sk_filter_free(filter->prog);
kfree(filter);
}
}
/* put_seccomp_filter - decrements the ref count of tsk->seccomp.filter */ /* put_seccomp_filter - decrements the ref count of tsk->seccomp.filter */
void put_seccomp_filter(struct task_struct *tsk) void put_seccomp_filter(struct task_struct *tsk)
{ {
...@@ -354,8 +383,7 @@ void put_seccomp_filter(struct task_struct *tsk) ...@@ -354,8 +383,7 @@ void put_seccomp_filter(struct task_struct *tsk)
while (orig && atomic_dec_and_test(&orig->usage)) { while (orig && atomic_dec_and_test(&orig->usage)) {
struct seccomp_filter *freeme = orig; struct seccomp_filter *freeme = orig;
orig = orig->prev; orig = orig->prev;
sk_filter_free(freeme->prog); seccomp_filter_free(freeme);
kfree(freeme);
} }
} }
...@@ -533,21 +561,30 @@ static long seccomp_set_mode_filter(unsigned int flags, ...@@ -533,21 +561,30 @@ static long seccomp_set_mode_filter(unsigned int flags,
const char __user *filter) const char __user *filter)
{ {
const unsigned long seccomp_mode = SECCOMP_MODE_FILTER; const unsigned long seccomp_mode = SECCOMP_MODE_FILTER;
struct seccomp_filter *prepared = NULL;
long ret = -EINVAL; long ret = -EINVAL;
/* Validate flags. */ /* Validate flags. */
if (flags != 0) if (flags != 0)
goto out; goto out;
/* Prepare the new filter before holding any locks. */
prepared = seccomp_prepare_user_filter(filter);
if (IS_ERR(prepared))
return PTR_ERR(prepared);
if (!seccomp_may_assign_mode(seccomp_mode)) if (!seccomp_may_assign_mode(seccomp_mode))
goto out; goto out;
ret = seccomp_attach_user_filter(filter); ret = seccomp_attach_filter(flags, prepared);
if (ret) if (ret)
goto out; goto out;
/* Do not free the successfully attached filter. */
prepared = NULL;
seccomp_assign_mode(seccomp_mode); seccomp_assign_mode(seccomp_mode);
out: out:
seccomp_filter_free(prepared);
return ret; return ret;
} }
#else #else
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment