Twelfth time's the charm! :)
This adds the ability for threads to request seccomp filter
synchronization across their thread group (at filter attach time).
For example, for Chrome to make sure graphic driver threads are fully
confined after seccomp filters have been attached.
To support this, locking on seccomp changes via thread-group-shared
sighand lock is introduced, along with refactoring of no_new_privs. Races
with thread creation are handled via delayed duplication of the seccomp
task struct field and cred_guard_mutex.
This includes a new syscall (instead of adding a new prctl option),
as suggested by Andy Lutomirski and Michael Kerrisk.
Thanks!
-Kees
v12:
- fixed bug where initial filter wouldn't allow TSYNC flag (drysdale)
- optimized thread loops (drysdale)
v11:
- updated writer locking commit log for clarity (luto)
- clarified writer lock thread flag setting comment (luto)
- inverted SECCOMP_FILTER_FLAG_MASK (luto)
- renamed is_acestor parameter (luto)
- added BUG_ON to catch currently impossible integer overflow (luto)
v10:
- dropped pending-kill checks (oleg)
- tweaked memory barriers (oleg)
v9:
- rearranged/split patches to make things more reviewable
- added use of cred_guard_mutex to solve exec race (oleg, luto)
- added barriers for TIF_SECCOMP vs seccomp.mode race (oleg, luto)
- fixed missed copying of nnp state after v8 refactor (oleg)
v8:
- drop use of tasklist_lock, appears redundant against sighand (oleg)
- reduced use of smp_load_acquire to logical minimum (oleg)
- change nnp to a task struct held atomic flags field (oleg, luto)
- drop needless irqflags changes in fork.c for holding sighand lock (oleg)
- cleaned up use of thread for-each loop (oleg)
- rearranged patch order to keep syscall changes adjacent
- added example code to manpage (mtk)
v7:
- rebase on Linus's tree (merged with network bpf changes)
- wrote manpage text documenting API (follows this series)
v6:
- switch from seccomp-specific lock to thread-group lock to gain atomicity
- implement seccomp syscall across all architectures with seccomp filter
- clean up sparse warnings around locking
v5:
- move includes around (drysdale)
- drop set_nnp return value (luto)
- use smp_load_acquire/store_release (luto)
- merge nnp changes to seccomp always, fewer ifdef (luto)
v4:
- cleaned up locking further, as noticed by David Drysdale
v3:
- added SECCOMP_EXT_ACT_FILTER for new filter install options
v2:
- reworked to avoid clone races
Separates the two mode setting paths to make things more readable with
fewer #ifdefs within function bodies.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <redacted>
Reviewed-by: Andy Lutomirski <redacted>
---
kernel/seccomp.c | 71 ++++++++++++++++++++++++++++++++++++------------------
1 file changed, 48 insertions(+), 23 deletions(-)
This changes the mode setting helper to allow threads to change the
seccomp mode from another thread. We must maintain barriers to keep
TIF_SECCOMP synchronized with the rest of the seccomp state.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
kernel/seccomp.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
@@ -435,12 +444,17 @@ static int mode1_syscalls_32[] = {int__secure_computing(intthis_syscall){-intmode=current->seccomp.mode;intexit_sig=0;int*syscall;u32ret;-switch(mode){+/*+*Makesurethatanychangestomodefromanotherthreadhave+*beenseenafterTIF_SECCOMPwasseen.+*/+rmb();++switch(current->seccomp.mode){caseSECCOMP_MODE_STRICT:syscall=mode1_syscalls;#ifdef CONFIG_COMPAT
@@ -545,7 +559,7 @@ static long seccomp_set_mode_strict(void)#ifdef TIF_NOTSCdisable_TSC();#endif-seccomp_assign_mode(seccomp_mode);+seccomp_assign_mode(current,seccomp_mode);ret=0;out:
@@ -595,7 +609,7 @@ static long seccomp_set_mode_filter(unsigned int flags,/* Do not free the successfully attached filter. */prepared=NULL;-seccomp_assign_mode(seccomp_mode);+seccomp_assign_mode(current,seccomp_mode);out:spin_unlock_irq(¤t->sighand->siglock);seccomp_filter_free(prepared);
Since seccomp transitions between threads requires updates to the
no_new_privs flag to be atomic, the flag must be part of an atomic flag
set. This moves the nnp flag into a separate task field, and introduces
accessors.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
fs/exec.c | 4 ++--
include/linux/sched.h | 18 +++++++++++++++---
kernel/seccomp.c | 2 +-
kernel/sys.c | 4 ++--
security/apparmor/domain.c | 4 ++--
5 files changed, 22 insertions(+), 10 deletions(-)
@@ -1307,13 +1307,12 @@ struct task_struct {*execve*/unsignedin_iowait:1;-/* task may not gain privileges */-unsignedno_new_privs:1;-/* Revert to default priority/policy when forking */unsignedsched_reset_on_fork:1;unsignedsched_contributes_to_load:1;+unsignedlongatomic_flags;/* Flags needing atomic access. */+pid_tpid;pid_ttgid;
@@ -1967,6 +1966,19 @@ static inline void memalloc_noio_restore(unsigned int flags)current->flags=(current->flags&~PF_MEMALLOC_NOIO)|flags;}+/* Per-process atomic flags. */+#define PFA_NO_NEW_PRIVS 0x00000001 /* May not gain new privileges. */++staticinlinebooltask_no_new_privs(structtask_struct*p)+{+returntest_bit(PFA_NO_NEW_PRIVS,&p->atomic_flags);+}++staticinlinevoidtask_set_no_new_privs(structtask_struct*p)+{+set_bit(PFA_NO_NEW_PRIVS,&p->atomic_flags);+}+/**task->jobctlflags*/
Applying restrictive seccomp filter programs to large or diverse
codebases often requires handling threads which may be started early in
the process lifetime (e.g., by code that is linked in). While it is
possible to apply permissive programs prior to process start up, it is
difficult to further restrict the kernel ABI to those threads after that
point.
This change adds a new seccomp syscall flag to SECCOMP_SET_MODE_FILTER for
synchronizing thread group seccomp filters at filter installation time.
When calling seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_TSYNC,
filter) an attempt will be made to synchronize all threads in current's
threadgroup to its new seccomp filter program. This is possible iff all
threads are using a filter that is an ancestor to the filter current is
attempting to synchronize to. NULL filters (where the task is running as
SECCOMP_MODE_NONE) are also treated as ancestors allowing threads to be
transitioned into SECCOMP_MODE_FILTER. If prctrl(PR_SET_NO_NEW_PRIVS,
...) has been set on the calling thread, no_new_privs will be set for
all synchronized threads too. On success, 0 is returned. On failure,
the pid of one of the failing threads will be returned and no filters
will have been applied.
The race conditions against another thread are:
- requesting TSYNC (already handled by sighand lock)
- performing a clone (already handled by sighand lock)
- changing its filter (already handled by sighand lock)
- calling exec (handled by cred_guard_mutex)
The clone case is assisted by the fact that new threads will have their
seccomp state duplicated from their parent before appearing on the tasklist.
Holding cred_guard_mutex means that seccomp filters cannot be assigned
while in the middle of another thread's exec (potentially bypassing
no_new_privs or similar). The call to de_thread() may kill threads waiting
for the mutex.
Changes across threads to the filter pointer includes a barrier.
Based on patches by Will Drewry.
Suggested-by: Julien Tinnes <redacted>
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
fs/exec.c | 2 +-
include/linux/seccomp.h | 2 +
include/uapi/linux/seccomp.h | 3 +
kernel/seccomp.c | 135 +++++++++++++++++++++++++++++++++++++++++-
4 files changed, 140 insertions(+), 2 deletions(-)
@@ -225,6 +226,114 @@ static inline void seccomp_assign_mode(struct task_struct *task,}#ifdef CONFIG_SECCOMP_FILTER+/* Returns 1 if the parent is an ancestor of the child. */+staticintis_ancestor(structseccomp_filter*parent,+structseccomp_filter*child)+{+/* NULL is the root ancestor. */+if(parent==NULL)+return1;+for(;child;child=child->prev)+if(child==parent)+return1;+return0;+}++/**+*seccomp_can_sync_threads:checksifallthreadscanbesynchronized+*+*Expectssighandandcred_guard_mutexlockstobeheld.+*+*Returns0onsuccess,-veonerror,orthepidofathreadwhichwas+*eithernotinthecorrectseccompmodeoritdidnothaveanancestral+*seccompfilter.+*/+staticinlinepid_tseccomp_can_sync_threads(void)+{+structtask_struct*thread,*caller;++BUG_ON(!mutex_is_locked(¤t->signal->cred_guard_mutex));+BUG_ON(!spin_is_locked(¤t->sighand->siglock));++/* Validate all threads being eligible for synchronization. */+caller=current;+for_each_thread(caller,thread){+pid_tfailed;++/* Skip current, since it is initiating the sync. */+if(thread==caller)+continue;++if(thread->seccomp.mode==SECCOMP_MODE_DISABLED||+(thread->seccomp.mode==SECCOMP_MODE_FILTER&&+is_ancestor(thread->seccomp.filter,+caller->seccomp.filter)))+continue;++/* Return the first thread that cannot be synchronized. */+failed=task_pid_vnr(thread);+/* If the pid cannot be resolved, then return -ESRCH */+if(unlikely(WARN_ON(failed==0)))+failed=-ESRCH;+returnfailed;+}++return0;+}++/**+*seccomp_sync_threads:setsallthreadstousecurrent'sfilter+*+*Expectssighandandcred_guard_mutexlockstobeheld,andfor+*seccomp_can_sync_threads()tohavereturnedsuccessalready+*withoutdroppingthelocks.+*+*/+staticinlinevoidseccomp_sync_threads(void)+{+structtask_struct*thread,*caller;++BUG_ON(!mutex_is_locked(¤t->signal->cred_guard_mutex));+BUG_ON(!spin_is_locked(¤t->sighand->siglock));++/* Synchronize all threads. */+caller=current;+for_each_thread(caller,thread){+/* Skip current, since it needs no changes. */+if(thread==caller)+continue;++/* Get a task reference for the new leaf node. */+get_seccomp_filter(caller);+/*+*Dropthetaskreferencetothesharedancestorsince+*current'spathwillholdareference.(Thisalso+*allowsaputbeforetheassignment.)+*/+put_seccomp_filter(thread);+smp_store_release(&thread->seccomp.filter,+caller->seccomp.filter);+/*+*Opttheotherthreadintoseccompifneeded.+*Asthreadsareconsideredtobetrust-realm+*equivalent(seeptrace_may_access),itissafeto+*allowonethreadtotransitiontheother.+*/+if(thread->seccomp.mode==SECCOMP_MODE_DISABLED){+/*+*Don'tletanunprivilegedtaskworkaround+*theno_new_privsrestrictionbycreating+*athreadthatsetsitup,entersseccomp,+*thendies.+*/+if(task_no_new_privs(caller))+task_set_no_new_privs(thread);++seccomp_assign_mode(thread,SECCOMP_MODE_FILTER);+}+}+}+/***seccomp_prepare_filter:Preparesaseccompfilterforuse.*@fprog:BPFprogramtoinstall
@@ -364,6 +473,15 @@ static long seccomp_attach_filter(unsigned int flags,if(total_insns>MAX_INSNS_PER_PATH)return-ENOMEM;+/* If thread sync has been requested, check that it is possible. */+if(flags&SECCOMP_FILTER_FLAG_TSYNC){+intret;++ret=seccomp_can_sync_threads();+if(ret)+returnret;+}+/**Ifthereisanexistingfilter,makeittheprevanddon'tdropits*taskreference.
@@ -371,6 +489,10 @@ static long seccomp_attach_filter(unsigned int flags,filter->prev=current->seccomp.filter;current->seccomp.filter=filter;+/* Now that the new filter is in place, synchronize to all threads. */+if(flags&SECCOMP_FILTER_FLAG_TSYNC)+seccomp_sync_threads();+return0;}
@@ -590,7 +712,7 @@ static long seccomp_set_mode_filter(unsigned int flags,longret=-EINVAL;/* Validate flags. */-if(flags!=0)+if(flags&~SECCOMP_FILTER_FLAG_MASK)return-EINVAL;/* Prepare the new filter before holding any locks. */
@@ -598,6 +720,14 @@ static long seccomp_set_mode_filter(unsigned int flags,if(IS_ERR(prepared))returnPTR_ERR(prepared);+/*+*MakesurewecannotchangeseccompornnpstateviaTSYNC+*whileanotherthreadisinthemiddleofcallingexec.+*/+if(flags&SECCOMP_FILTER_FLAG_TSYNC&&+mutex_lock_killable(¤t->signal->cred_guard_mutex))+gotoout_free;+spin_lock_irq(¤t->sighand->siglock);if(!seccomp_may_assign_mode(seccomp_mode))
@@ -612,6 +742,9 @@ static long seccomp_set_mode_filter(unsigned int flags,seccomp_assign_mode(current,seccomp_mode);out:spin_unlock_irq(¤t->sighand->siglock);+if(flags&SECCOMP_FILTER_FLAG_TSYNC)+mutex_unlock(¤t->signal->cred_guard_mutex);+out_free:seccomp_filter_free(prepared);returnret;}
This adds the new "seccomp" syscall with both an "operation" and "flags"
parameter for future expansion. The third argument is a pointer value,
used with the SECCOMP_SET_MODE_FILTER operation. Currently, flags must
be 0. This is functionally equivalent to prctl(PR_SET_SECCOMP, ...).
In addition to the TSYNC flag later in this patch series, there is a
non-zero chance that this syscall could be used for configuring a fixed
argument area for seccomp-tracer-aware processes to pass syscall arguments
in the future. Hence, the use of "seccomp" not simply "seccomp_add_filter"
for this syscall. Additionally, this syscall uses operation, flags,
and user pointer for arguments because strictly passing arguments via
a user pointer would mean seccomp itself would be unable to trivially
filter the seccomp syscall itself.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
arch/Kconfig | 1 +
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 1 +
include/linux/syscalls.h | 2 ++
include/uapi/asm-generic/unistd.h | 4 ++-
include/uapi/linux/seccomp.h | 4 +++
kernel/seccomp.c | 55 +++++++++++++++++++++++++++++++++----
kernel/sys_ni.c | 3 ++
8 files changed, 65 insertions(+), 6 deletions(-)
@@ -323,6 +323,7 @@ 314 common sched_setattr sys_sched_setattr 315 common sched_getattr sys_sched_getattr 316 common renameat2 sys_renameat2+317 common seccomp sys_seccomp # # x32-specific system call numbers start at 512 to avoid cache impact
@@ -544,12 +551,35 @@ out:returnret;}#else-staticinlinelongseccomp_set_mode_filter(char__user*filter)+staticinlinelongseccomp_set_mode_filter(unsignedintflags,+constchar__user*filter){return-EINVAL;}#endif+/* Common entry point for both prctl and syscall. */+staticlongdo_seccomp(unsignedintop,unsignedintflags,+constchar__user*uargs)+{+switch(op){+caseSECCOMP_SET_MODE_STRICT:+if(flags!=0||uargs!=NULL)+return-EINVAL;+returnseccomp_set_mode_strict();+caseSECCOMP_SET_MODE_FILTER:+returnseccomp_set_mode_filter(flags,uargs);+default:+return-EINVAL;+}+}++SYSCALL_DEFINE3(seccomp,unsignedint,op,unsignedint,flags,+constchar__user*,uargs)+{+returndo_seccomp(op,flags,uargs);+}+/***prctl_set_seccomp:configurescurrent->seccomp.mode*@seccomp_mode:requestedmodetouse
@@ -559,12 +589,27 @@ static inline long seccomp_set_mode_filter(char __user *filter)*/longprctl_set_seccomp(unsignedlongseccomp_mode,char__user*filter){+unsignedintop;+char__user*uargs;+switch(seccomp_mode){caseSECCOMP_MODE_STRICT:-returnseccomp_set_mode_strict();+op=SECCOMP_SET_MODE_STRICT;+/*+*Settingstrictmodethroughprctlalwaysignoredfilter,+*somakesureitisalwaysNULLheretopasstheinternal+*checkindo_seccomp().+*/+uargs=NULL;+break;caseSECCOMP_MODE_FILTER:-returnseccomp_set_mode_filter(filter);+op=SECCOMP_SET_MODE_FILTER;+uargs=filter;+break;default:return-EINVAL;}++/* prctl interface doesn't have flags, so they are always zero. */+returndo_seccomp(op,0,uargs);}
Normally, task_struct.seccomp.filter is only ever read or modified by
the task that owns it (current). This property aids in fast access
during system call filtering as read access is lockless.
Updating the pointer from another task, however, opens up race
conditions. To allow cross-thread filter pointer updates, writes to the
seccomp fields are now protected by the sighand spinlock (which is shared
by all threads in the thread group). Read access remains lockless because
pointer updates themselves are atomic. However, writes (or cloning)
often entail additional checking (like maximum instruction counts)
which require locking to perform safely.
In the case of cloning threads, the child is invisible to the system
until it enters the task list. To make sure a child can't be cloned from
a thread and left in a prior state, seccomp duplication is additionally
moved under the sighand lock. Then parent and child are certain have
the same seccomp state when they exit the lock.
Based on patches by Will Drewry and David Drysdale.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <redacted>
Reviewed-by: Andy Lutomirski <redacted>
---
include/linux/seccomp.h | 6 +++---
kernel/fork.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++-
kernel/seccomp.c | 16 +++++++++++++++-
3 files changed, 66 insertions(+), 5 deletions(-)
@@ -1081,6 +1090,39 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)return0;}+staticvoidcopy_seccomp(structtask_struct*p)+{+#ifdef CONFIG_SECCOMP+/*+*Mustbecalledwithsighand->lockheld,whichiscommonto+*allthreadsinthegroup.Holdingcred_guard_mutexisnot+*neededbecausethisnewtaskisnotyetrunningandcannot+*beracingexec.+*/+BUG_ON(!spin_is_locked(¤t->sighand->siglock));++/* Ref-count the new filter user, and assign it. */+get_seccomp_filter(current);+p->seccomp=current->seccomp;++/*+*Explicitlyenableno_new_privshereincaseitgotset+*betweenthetask_structbeingduplicatedandholdingthe+*sighandlock.Theseccompstateandnnpmustbeinsync.+*/+if(task_no_new_privs(current))+task_set_no_new_privs(p);++/*+*Iftheparentgainedaseccompmodeaftercopyingthread+*flagsandbetweenbeforeweheldthesighandlock,wehave+*tomanuallyenabletheseccompthreadflaghere.+*/+if(p->seccomp.mode!=SECCOMP_MODE_DISABLED)+set_tsk_thread_flag(p,TIF_SECCOMP);+#endif+}+SYSCALL_DEFINE1(set_tid_address,int__user*,tidptr){current->clear_child_tid=tidptr;
@@ -1196,7 +1238,6 @@ static struct task_struct *copy_process(unsigned long clone_flags,gotofork_out;ftrace_graph_init_task(p);-get_seccomp_filter(p);rt_mutex_init_task(p);
@@ -1437,6 +1478,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,spin_lock(¤t->sighand->siglock);/*+*Copyseccompdetailsexplicitlyhere,incasetheywerechanged+*beforeholdingsighandlock.+*/+copy_seccomp(p);++/**Processgroupandsessionsignalsneedtobedeliveredtojustthe*parentbeforetheforkorboththeparentandthechildafterthe*fork.Restartifasignalcomesinbeforeweaddthenewprocessto
@@ -340,6 +346,8 @@ static long seccomp_attach_filter(unsigned int flags,unsignedlongtotal_insns;structseccomp_filter*walker;+BUG_ON(!spin_is_locked(¤t->sighand->siglock));+/* Validate resulting filter length. */total_insns=filter->prog->len;for(walker=current->seccomp.filter;walker;walker=walker->prev)
@@ -529,6 +537,8 @@ static long seccomp_set_mode_strict(void)constunsignedlongseccomp_mode=SECCOMP_MODE_STRICT;longret=-EINVAL;+spin_lock_irq(¤t->sighand->siglock);+if(!seccomp_may_assign_mode(seccomp_mode))gotoout;
@@ -539,6 +549,7 @@ static long seccomp_set_mode_strict(void)ret=0;out:+spin_unlock_irq(¤t->sighand->siglock);returnret;}
@@ -566,13 +577,15 @@ static long seccomp_set_mode_filter(unsigned int flags,/* Validate flags. */if(flags!=0)-gotoout;+return-EINVAL;/* Prepare the new filter before holding any locks. */prepared=seccomp_prepare_user_filter(filter);if(IS_ERR(prepared))returnPTR_ERR(prepared);+spin_lock_irq(¤t->sighand->siglock);+if(!seccomp_may_assign_mode(seccomp_mode))gotoout;
@@ -584,6 +597,7 @@ static long seccomp_set_mode_filter(unsigned int flags,seccomp_assign_mode(seccomp_mode);out:+spin_unlock_irq(¤t->sighand->siglock);seccomp_filter_free(prepared);returnret;}
In preparation for having other callers of the seccomp mode setting
logic, split the prctl entry point away from the core logic that performs
seccomp mode setting.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
kernel/seccomp.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
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: Kees Cook <redacted>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
kernel/seccomp.c | 97 +++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 67 insertions(+), 30 deletions(-)
@@ -213,27 +213,23 @@ static inline void seccomp_assign_mode(unsigned long seccomp_mode)#ifdef CONFIG_SECCOMP_FILTER/**-*seccomp_attach_filter:Attachesaseccompfiltertocurrent.+*seccomp_prepare_filter:Preparesaseccompfilterforuse.*@fprog:BPFprogramtoinstall*-*Returns0onsuccessoranerrnoonfailure.+*ReturnsfilteronsuccessoranERR_PTRonfailure.*/-staticlongseccomp_attach_filter(structsock_fprog*fprog)+staticstructseccomp_filter*seccomp_prepare_filter(structsock_fprog*fprog){structseccomp_filter*filter;-unsignedlongfp_size=fprog->len*sizeof(structsock_filter);-unsignedlongtotal_insns=fprog->len;+unsignedlongfp_size;structsock_filter*fp;intnew_len;longret;if(fprog->len==0||fprog->len>BPF_MAXINSNS)-return-EINVAL;--for(filter=current->seccomp.filter;filter;filter=filter->prev)-total_insns+=filter->prog->len+4;/* include a 4 instr penalty */-if(total_insns>MAX_INSNS_PER_PATH)-return-ENOMEM;+returnERR_PTR(-EINVAL);+BUG_ON(INT_MAX/fprog->len<sizeof(structsock_filter));+fp_size=fprog->len*sizeof(structsock_filter);/**Installingaseccompfilterrequiresthatthetaskhas
@@ -244,11 +240,11 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)if(!task_no_new_privs(current)&&security_capable_noaudit(current_cred(),current_user_ns(),CAP_SYS_ADMIN)!=0)-return-EACCES;+returnERR_PTR(-EACCES);fp=kzalloc(fp_size,GFP_KERNEL|__GFP_NOWARN);if(!fp)-return-ENOMEM;+returnERR_PTR(-ENOMEM);/* Copy the instructions from fprog. */ret=-EFAULT;
@@ -292,13 +288,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)sk_filter_select_runtime(filter->prog);-/*-*Ifthereisanexistingfilter,makeittheprevanddon'tdropits-*taskreference.-*/-filter->prev=current->seccomp.filter;-current->seccomp.filter=filter;-return0;+returnfilter;free_filter_prog:kfree(filter->prog);
@@ -533,21 +561,30 @@ static long seccomp_set_mode_filter(unsigned int flags,constchar__user*filter){constunsignedlongseccomp_mode=SECCOMP_MODE_FILTER;+structseccomp_filter*prepared=NULL;longret=-EINVAL;/* Validate flags. */if(flags!=0)gotoout;+/* Prepare the new filter before holding any locks. */+prepared=seccomp_prepare_user_filter(filter);+if(IS_ERR(prepared))+returnPTR_ERR(prepared);+if(!seccomp_may_assign_mode(seccomp_mode))gotoout;-ret=seccomp_attach_user_filter(filter);+ret=seccomp_attach_filter(flags,prepared);if(ret)gotoout;+/* Do not free the successfully attached filter. */+prepared=NULL;seccomp_assign_mode(seccomp_mode);out:+seccomp_filter_free(prepared);returnret;}#else
From: James Morris <jmorris@namei.org> Date: 2014-07-18 03:27:36
On Thu, 17 Jul 2014, Kees Cook wrote:
Twelfth time's the charm! :)
Btw, there doesn't seem to be an official seccomp maintainer. Kees, would
you like to volunteer for this? If so, send in a patch for MAINTAINERS,
and set up a git tree for me to pull from.
--
James Morris
[off-list ref]
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-07-18 17:17:31
On Thu, Jul 17, 2014 at 8:26 PM, James Morris [off-list ref] wrote:
On Thu, 17 Jul 2014, Kees Cook wrote:
quoted
Twelfth time's the charm! :)
Btw, there doesn't seem to be an official seccomp maintainer. Kees, would
you like to volunteer for this? If so, send in a patch for MAINTAINERS,
and set up a git tree for me to pull from.
*snicker* :)
Kees, if you take on this awesome responsibility, should I send you a
rebased version of the fastpath stuff? If so, I think that the
arch-neutral part should go in through your shiny new tree (once it's
reviewed to your satisfaction), and I'll ask hpa to pick up the x86
part.
I'd volunteer to be a "R:eviewer", but I don't think that the R tag
has made it into MAINTAINERS yet.
--Andy
On Fri, Jul 18, 2014 at 10:17 AM, Andy Lutomirski [off-list ref] wrote:
On Thu, Jul 17, 2014 at 8:26 PM, James Morris [off-list ref] wrote:
quoted
On Thu, 17 Jul 2014, Kees Cook wrote:
quoted
Twelfth time's the charm! :)
Btw, there doesn't seem to be an official seccomp maintainer. Kees, would
you like to volunteer for this? If so, send in a patch for MAINTAINERS,
and set up a git tree for me to pull from.
Sure thing, I'll get this set up now. I wonder if I should include the
glob "arch/*/kernel/ptrace.c" in the MAINTAINERS entry. :P
*snicker* :)
Kees, if you take on this awesome responsibility, should I send you a
rebased version of the fastpath stuff? If so, I think that the
arch-neutral part should go in through your shiny new tree (once it's
reviewed to your satisfaction), and I'll ask hpa to pick up the x86
part.
Yeah, that sounds perfect.
I'd volunteer to be a "R:eviewer", but I don't think that the R tag
has made it into MAINTAINERS yet.
I'd love to have you listed. :)
-Kees
--
Kees Cook
Chrome OS Security
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-07-18 18:51:43
On Fri, Jul 18, 2014 at 11:13 AM, Kees Cook [off-list ref] wrote:
On Fri, Jul 18, 2014 at 10:17 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Thu, Jul 17, 2014 at 8:26 PM, James Morris [off-list ref] wrote:
quoted
On Thu, 17 Jul 2014, Kees Cook wrote:
quoted
Twelfth time's the charm! :)
Btw, there doesn't seem to be an official seccomp maintainer. Kees, would
you like to volunteer for this? If so, send in a patch for MAINTAINERS,
and set up a git tree for me to pull from.
Sure thing, I'll get this set up now. I wonder if I should include the
glob "arch/*/kernel/ptrace.c" in the MAINTAINERS entry. :P
You might want to convince some arch maintainers first, and that's
like herding cats. Or Oleg, at least. Also, do you really want to be
asked to deal with the never-ending stream of bugs that people find in
files matching that glob?
quoted
*snicker* :)
Kees, if you take on this awesome responsibility, should I send you a
rebased version of the fastpath stuff? If so, I think that the
arch-neutral part should go in through your shiny new tree (once it's
reviewed to your satisfaction), and I'll ask hpa to pick up the x86
part.
Yeah, that sounds perfect.
quoted
I'd volunteer to be a "R:eviewer", but I don't think that the R tag
has made it into MAINTAINERS yet.
I'd love to have you listed. :)
Feel free to pester me once "R" appears in case I forget.
You might be able to convince me to co-maintain some day. Hmm.
--Andy
On Fri, Jul 18, 2014 at 11:51 AM, Andy Lutomirski [off-list ref] wrote:
On Fri, Jul 18, 2014 at 11:13 AM, Kees Cook [off-list ref] wrote:
quoted
On Fri, Jul 18, 2014 at 10:17 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Thu, Jul 17, 2014 at 8:26 PM, James Morris [off-list ref] wrote:
quoted
On Thu, 17 Jul 2014, Kees Cook wrote:
quoted
Twelfth time's the charm! :)
Btw, there doesn't seem to be an official seccomp maintainer. Kees, would
you like to volunteer for this? If so, send in a patch for MAINTAINERS,
and set up a git tree for me to pull from.
Sure thing, I'll get this set up now. I wonder if I should include the
glob "arch/*/kernel/ptrace.c" in the MAINTAINERS entry. :P
You might want to convince some arch maintainers first, and that's
like herding cats. Or Oleg, at least. Also, do you really want to be
asked to deal with the never-ending stream of bugs that people find in
files matching that glob?
Yeah, I've opted for "K: \bsecure_computing" Seemed like a decent middle-ground.
quoted
quoted
*snicker* :)
Kees, if you take on this awesome responsibility, should I send you a
rebased version of the fastpath stuff? If so, I think that the
arch-neutral part should go in through your shiny new tree (once it's
reviewed to your satisfaction), and I'll ask hpa to pick up the x86
part.
Yeah, that sounds perfect.
quoted
I'd volunteer to be a "R:eviewer", but I don't think that the R tag
has made it into MAINTAINERS yet.
I'd love to have you listed. :)
Feel free to pester me once "R" appears in case I forget.
I will indeed! :)
-Kees
You might be able to convince me to co-maintain some day. Hmm.
--Andy