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.
This includes a new syscall (instead of adding a new prctl option),
as suggested by Andy Lutomirski and Michael Kerrisk.
Thanks!
-Kees
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
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 are against another thread requesting TSYNC, another
thread performing a clone, and another thread changing its filter. The
sighand lock is sufficient for these cases. 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.
Based on patches by Will Drewry.
Suggested-by: Julien Tinnes <redacted>
Signed-off-by: Kees Cook <redacted>
---
include/linux/seccomp.h | 2 +
include/uapi/linux/seccomp.h | 3 ++
kernel/seccomp.c | 115 +++++++++++++++++++++++++++++++++++++++++-
3 files changed, 119 insertions(+), 1 deletion(-)
@@ -218,6 +219,105 @@ static inline void seccomp_assign_mode(struct task_struct *task,}#ifdef CONFIG_SECCOMP_FILTER+/* Returns 1 if the candidate is an ancestor. */+staticintis_ancestor(structseccomp_filter*candidate,+structseccomp_filter*child)+{+/* NULL is the root ancestor. */+if(candidate==NULL)+return1;+for(;child;child=child->prev)+if(child==candidate)+return1;+return0;+}++/**+*seccomp_can_sync_threads:checksifallthreadscanbesynchronized+*+*Expectscurrent->sighand->siglocktobeheld.+*+*Returns0onsuccess,-veonerror,orthepidofathreadwhichwas+*eithernotinthecorrectseccompmodeoritdidnothaveanancestral+*seccompfilter.+*/+staticpid_tseccomp_can_sync_threads(void)+{+structtask_struct*thread,*caller;++BUG_ON(!spin_is_locked(¤t->sighand->siglock));++if(current->seccomp.mode!=SECCOMP_MODE_FILTER)+return-EACCES;++/* Validate all threads being eligible for synchronization. */+caller=current;+for_each_thread(caller,thread){+pid_tfailed;++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+*+*Expectscurrent->sighand->siglocktobeheld,+*andforseccomp_can_sync_threads()tohavereturnedsuccessalready+*withoutdroppingthelocks.+*+*/+staticvoidseccomp_sync_threads(void)+{+structtask_struct*thread,*caller;++BUG_ON(!spin_is_locked(¤t->sighand->siglock));++/* Synchronize all threads. */+caller=current;+for_each_thread(caller,thread){+/* Get a task reference for the new leaf node. */+get_seccomp_filter(caller);+/*+*Dropthetaskreferencetothesharedancestorsince+*current'spathwillholdareference.(Thisalso+*allowsaputbeforetheassignment.)+*/+put_seccomp_filter(thread);+thread->seccomp.filter=caller->seccomp.filter;+/* Opt the other thread into seccomp if needed.+*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
@@ -349,7 +449,7 @@ static long seccomp_attach_filter(unsigned int flags,BUG_ON(!spin_is_locked(¤t->sighand->siglock));/* Validate flags. */-if(flags!=0)+if(flags&SECCOMP_FILTER_FLAG_MASK)return-EINVAL;/* Validate resulting filter length. */
@@ -359,6 +459,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.
@@ -366,6 +475,10 @@ static long seccomp_attach_filter(unsigned int flags,filter->prev=current->seccomp.filter;smp_store_release(¤t->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;}
+static void seccomp_sync_threads(void)
+{
+ struct task_struct *thread, *caller;
+
+ BUG_ON(!spin_is_locked(¤t->sighand->siglock));
+
+ /* Synchronize all threads. */
+ caller = current;
+ for_each_thread(caller, thread) {
+ /* Get a task reference for the new leaf node. */
+ get_seccomp_filter(caller);
+ /*
+ * Drop the task reference to the shared ancestor since
+ * current's path will hold a reference. (This also
+ * allows a put before the assignment.)
+ */
+ put_seccomp_filter(thread);
+ thread->seccomp.filter = caller->seccomp.filter;
+ /* Opt the other thread into seccomp if needed.
+ * As threads are considered to be trust-realm
+ * equivalent (see ptrace_may_access), it is safe to
+ * allow one thread to transition the other.
+ */
+ if (thread->seccomp.mode == SECCOMP_MODE_DISABLED) {
+ /*
+ * Don't let an unprivileged task work around
+ * the no_new_privs restriction by creating
+ * a thread that sets it up, enters seccomp,
+ * then dies.
+ */
+ if (task_no_new_privs(caller))
+ task_set_no_new_privs(thread);
+
+ seccomp_assign_mode(thread, SECCOMP_MODE_FILTER);
+ }
+ }
+}
OK, personally I think this all make sense. I even think that perhaps
SECCOMP_FILTER_FLAG_TSYNC should allow filter == NULL, a thread might
want to "sync" without adding the new filter, but this is minor/offtopic.
But. Doesn't this change add the new security hole?
Obviously, we should not allow to install a filter and then (say) exec
a suid binary, that is why we have no_new_privs/LSM_UNSAFE_NO_NEW_PRIVS.
But what if "thread->seccomp.filter = caller->seccomp.filter" races with
any user of task_no_new_privs() ? Say, suppose this thread has already
passed check_unsafe_exec/etc and it is going to exec the suid binary?
Oleg.
On Wed, Jun 25, 2014 at 7:21 AM, Oleg Nesterov [off-list ref] wrote:
On 06/24, Kees Cook wrote:
quoted
+static void seccomp_sync_threads(void)
+{
+ struct task_struct *thread, *caller;
+
+ BUG_ON(!spin_is_locked(¤t->sighand->siglock));
+
+ /* Synchronize all threads. */
+ caller = current;
+ for_each_thread(caller, thread) {
+ /* Get a task reference for the new leaf node. */
+ get_seccomp_filter(caller);
+ /*
+ * Drop the task reference to the shared ancestor since
+ * current's path will hold a reference. (This also
+ * allows a put before the assignment.)
+ */
+ put_seccomp_filter(thread);
+ thread->seccomp.filter = caller->seccomp.filter;
+ /* Opt the other thread into seccomp if needed.
+ * As threads are considered to be trust-realm
+ * equivalent (see ptrace_may_access), it is safe to
+ * allow one thread to transition the other.
+ */
+ if (thread->seccomp.mode == SECCOMP_MODE_DISABLED) {
+ /*
+ * Don't let an unprivileged task work around
+ * the no_new_privs restriction by creating
+ * a thread that sets it up, enters seccomp,
+ * then dies.
+ */
+ if (task_no_new_privs(caller))
+ task_set_no_new_privs(thread);
+
+ seccomp_assign_mode(thread, SECCOMP_MODE_FILTER);
+ }
+ }
+}
OK, personally I think this all make sense. I even think that perhaps
SECCOMP_FILTER_FLAG_TSYNC should allow filter == NULL, a thread might
want to "sync" without adding the new filter, but this is minor/offtopic.
But. Doesn't this change add a new security hole?
Obviously, we should not allow to install a filter and then (say) exec
a suid binary, that is why we have no_new_privs/LSM_UNSAFE_NO_NEW_PRIVS.
But what if "thread->seccomp.filter = caller->seccomp.filter" races with
any user of task_no_new_privs() ? Say, suppose this thread has already
passed check_unsafe_exec/etc and it is going to exec the suid binary?
Oh, ew. Yeah. It looks like there's a cred lock to be held to combat this?
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
-Kees
--
Kees Cook
Chrome OS Security
On Wed, Jun 25, 2014 at 7:21 AM, Oleg Nesterov [off-list ref] wrote:
quoted
But. Doesn't this change add a new security hole?
Obviously, we should not allow to install a filter and then (say) exec
a suid binary, that is why we have no_new_privs/LSM_UNSAFE_NO_NEW_PRIVS.
But what if "thread->seccomp.filter = caller->seccomp.filter" races with
any user of task_no_new_privs() ? Say, suppose this thread has already
passed check_unsafe_exec/etc and it is going to exec the suid binary?
Oh, ew. Yeah. It looks like there's a cred lock to be held to combat this?
Yes, cred_guard_mutex looks like an obvious choice... Hmm, but somehow
initially I thought that the fix won't be simple. Not sure why.
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
But I was also worried that task_no_new_privs(current) is no longer stable
inside the syscall paths, perhaps this is what you meant? However I do not
see something bad here... And this has nothing to do with the race above.
Also. Even ignoring no_new_privs, SECCOMP_FILTER_FLAG_TSYNC is not atomic
and we can do nothing with this fact (unless it try to freeze the thread
group somehow), perhaps it makes sense to document this somehow.
I mean, suppose you want to ensure write-to-file is not possible, so you
do seccomp(SECCOMP_FILTER_FLAG_TSYNC, nack_write_to_file_filter). You can't
assume that this has effect right after seccomp() returns, this can obviously
race with a sub-thread which has already entered sys_write().
Once again, I am not arguing, just I think it makes sense to at least mention
the limitations during the discussion.
Oleg.
On Wed, Jun 25, 2014 at 9:52 AM, Oleg Nesterov [off-list ref] wrote:
On 06/25, Kees Cook wrote:
quoted
On Wed, Jun 25, 2014 at 7:21 AM, Oleg Nesterov [off-list ref] wrote:
quoted
But. Doesn't this change add a new security hole?
Obviously, we should not allow to install a filter and then (say) exec
a suid binary, that is why we have no_new_privs/LSM_UNSAFE_NO_NEW_PRIVS.
But what if "thread->seccomp.filter = caller->seccomp.filter" races with
any user of task_no_new_privs() ? Say, suppose this thread has already
passed check_unsafe_exec/etc and it is going to exec the suid binary?
Oh, ew. Yeah. It looks like there's a cred lock to be held to combat this?
Yes, cred_guard_mutex looks like an obvious choice... Hmm, but somehow
initially I thought that the fix won't be simple. Not sure why.
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
quoted
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
Instead of having TSYNC change the nnp bit, it can set a new flag, say:
task->seccomp.flags |= SECCOMP_NEEDS_NNP;
This would be set along with seccomp.mode, seccomp.filter, and
TIF_SECCOMP. Then, during the next secure_computing() call that thread
makes, it would check the flag:
if (task->seccomp.flags & SECCOMP_NEEDS_NNP)
task->nnp = 1;
This means that nnp couldn't change in the middle of a running syscall.
Hmmm. Perhaps this doesn't solve anything, though? Perhaps my proposal
above would actually make things worse, since now we'd have a thread
with seccomp set up, and no nnp. If it was in the middle of exec,
we're still causing a problem.
I think we'd also need a way to either delay the seccomp changes, or
to notice this condition during exec. Bleh.
What actually happens with a multi-threaded process calls exec? I
assume all the other threads are destroyed?
But I was also worried that task_no_new_privs(current) is no longer stable
inside the syscall paths, perhaps this is what you meant? However I do not
see something bad here... And this has nothing to do with the race above.
Also. Even ignoring no_new_privs, SECCOMP_FILTER_FLAG_TSYNC is not atomic
and we can do nothing with this fact (unless it try to freeze the thread
group somehow), perhaps it makes sense to document this somehow.
I mean, suppose you want to ensure write-to-file is not possible, so you
do seccomp(SECCOMP_FILTER_FLAG_TSYNC, nack_write_to_file_filter). You can't
assume that this has effect right after seccomp() returns, this can obviously
race with a sub-thread which has already entered sys_write().
Once again, I am not arguing, just I think it makes sense to at least mention
the limitations during the discussion.
Right -- this is an accepted limitation. I will call it out
specifically in the man-page; that's a good idea.
-Kees
--
Kees Cook
Chrome OS Security
On Wed, Jun 25, 2014 at 9:52 AM, Oleg Nesterov [off-list ref] wrote:
quoted
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
quoted
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
Instead of having TSYNC change the nnp bit, it can set a new flag, say:
task->seccomp.flags |= SECCOMP_NEEDS_NNP;
This would be set along with seccomp.mode, seccomp.filter, and
TIF_SECCOMP. Then, during the next secure_computing() call that thread
makes, it would check the flag:
if (task->seccomp.flags & SECCOMP_NEEDS_NNP)
task->nnp = 1;
This means that nnp couldn't change in the middle of a running syscall.
Aha, so you were worried about the same thing. Not sure we need this,
but at least I understand you and...
Hmmm. Perhaps this doesn't solve anything, though? Perhaps my proposal
above would actually make things worse, since now we'd have a thread
with seccomp set up, and no nnp. If it was in the middle of exec,
we're still causing a problem.
Yes ;)
I think we'd also need a way to either delay the seccomp changes, or
to notice this condition during exec. Bleh.
Hmm. confused again,
What actually happens with a multi-threaded process calls exec? I
assume all the other threads are destroyed?
Yes. But this is the point-of-no-return, de_thread() is called after the execing
thared has already passed (say) check_unsafe_exec().
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
Oleg.
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-25 17:41:14
On Wed, Jun 25, 2014 at 10:24 AM, Oleg Nesterov [off-list ref] wrote:
On 06/25, Kees Cook wrote:
quoted
On Wed, Jun 25, 2014 at 9:52 AM, Oleg Nesterov [off-list ref] wrote:
quoted
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
quoted
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
Instead of having TSYNC change the nnp bit, it can set a new flag, say:
task->seccomp.flags |= SECCOMP_NEEDS_NNP;
This would be set along with seccomp.mode, seccomp.filter, and
TIF_SECCOMP. Then, during the next secure_computing() call that thread
makes, it would check the flag:
if (task->seccomp.flags & SECCOMP_NEEDS_NNP)
task->nnp = 1;
This means that nnp couldn't change in the middle of a running syscall.
Aha, so you were worried about the same thing. Not sure we need this,
but at least I understand you and...
quoted
Hmmm. Perhaps this doesn't solve anything, though? Perhaps my proposal
above would actually make things worse, since now we'd have a thread
with seccomp set up, and no nnp. If it was in the middle of exec,
we're still causing a problem.
Yes ;)
quoted
I think we'd also need a way to either delay the seccomp changes, or
to notice this condition during exec. Bleh.
Hmm. confused again,
quoted
What actually happens with a multi-threaded process calls exec? I
assume all the other threads are destroyed?
Yes. But this is the point-of-no-return, de_thread() is called after the execing
thared has already passed (say) check_unsafe_exec().
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
If you rely on this, then please fix this comment in fs/exec.c:
/*
* determine how safe it is to execute the proposed program
* - the caller must hold ->cred_guard_mutex to protect against
* PTRACE_ATTACH
*/
static void check_unsafe_exec(struct linux_binprm *bprm)
It sounds like cred_guard_mutex is there for exactly this reason :)
--Andy
On Wed, Jun 25, 2014 at 10:24 AM, Oleg Nesterov [off-list ref] wrote:
On 06/25, Kees Cook wrote:
quoted
On Wed, Jun 25, 2014 at 9:52 AM, Oleg Nesterov [off-list ref] wrote:
quoted
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
quoted
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
Instead of having TSYNC change the nnp bit, it can set a new flag, say:
task->seccomp.flags |= SECCOMP_NEEDS_NNP;
This would be set along with seccomp.mode, seccomp.filter, and
TIF_SECCOMP. Then, during the next secure_computing() call that thread
makes, it would check the flag:
if (task->seccomp.flags & SECCOMP_NEEDS_NNP)
task->nnp = 1;
This means that nnp couldn't change in the middle of a running syscall.
Aha, so you were worried about the same thing. Not sure we need this,
but at least I understand you and...
quoted
Hmmm. Perhaps this doesn't solve anything, though? Perhaps my proposal
above would actually make things worse, since now we'd have a thread
with seccomp set up, and no nnp. If it was in the middle of exec,
we're still causing a problem.
Yes ;)
quoted
I think we'd also need a way to either delay the seccomp changes, or
to notice this condition during exec. Bleh.
Hmm. confused again,
I mean to suggest that the tsync changes would be stored in each
thread, but somewhere other than the true seccomp struct, but with
TIF_SECCOMP set. When entering secure_computing(), current would check
for the "changes to sync", and apply them, then start the syscall. In
this way, we can never race a syscall (like exec).
quoted
What actually happens with a multi-threaded process calls exec? I
assume all the other threads are destroyed?
Yes. But this is the point-of-no-return, de_thread() is called after the execing
thared has already passed (say) check_unsafe_exec().
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
I can't tell yet. I'm still trying to understand the order of
operations here. It looks like de_thread() takes the sighand lock.
do_execve_common does:
prepare_bprm_creds (takes cred_guard_mutex)
check_unsafe_exec (checks nnp to set LSM_UNSAFE_NO_NEW_PRIVS)
prepare_binprm (handles suid escalation, checks nnp separately)
security_bprm_set_creds (checks LSM_UNSAFE_NO_NEW_PRIVS)
exec_binprm
load_elf_binary
flush_old_exec
de_thread (takes and releases sighand->lock)
install_exec_creds (releases cred_guard_mutex)
I don't see a way to use cred_guard_mutex during tsync (which holds
sighand->lock) without dead-locking. What were you considering here?
-Kees
--
Kees Cook
Chrome OS Security
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-25 18:10:06
On Wed, Jun 25, 2014 at 10:57 AM, Kees Cook [off-list ref] wrote:
On Wed, Jun 25, 2014 at 10:24 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Kees Cook wrote:
quoted
On Wed, Jun 25, 2014 at 9:52 AM, Oleg Nesterov [off-list ref] wrote:
quoted
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
quoted
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
Instead of having TSYNC change the nnp bit, it can set a new flag, say:
task->seccomp.flags |= SECCOMP_NEEDS_NNP;
This would be set along with seccomp.mode, seccomp.filter, and
TIF_SECCOMP. Then, during the next secure_computing() call that thread
makes, it would check the flag:
if (task->seccomp.flags & SECCOMP_NEEDS_NNP)
task->nnp = 1;
This means that nnp couldn't change in the middle of a running syscall.
Aha, so you were worried about the same thing. Not sure we need this,
but at least I understand you and...
quoted
Hmmm. Perhaps this doesn't solve anything, though? Perhaps my proposal
above would actually make things worse, since now we'd have a thread
with seccomp set up, and no nnp. If it was in the middle of exec,
we're still causing a problem.
Yes ;)
quoted
I think we'd also need a way to either delay the seccomp changes, or
to notice this condition during exec. Bleh.
Hmm. confused again,
I mean to suggest that the tsync changes would be stored in each
thread, but somewhere other than the true seccomp struct, but with
TIF_SECCOMP set. When entering secure_computing(), current would check
for the "changes to sync", and apply them, then start the syscall. In
this way, we can never race a syscall (like exec).
I'm not sure that helps. If you set a pending filter part-way through
exec, and exec copies that pending filter but doesn't notice NNP, then
there's an exploitable race.
quoted
quoted
What actually happens with a multi-threaded process calls exec? I
assume all the other threads are destroyed?
Yes. But this is the point-of-no-return, de_thread() is called after the execing
thared has already passed (say) check_unsafe_exec().
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
I can't tell yet. I'm still trying to understand the order of
operations here. It looks like de_thread() takes the sighand lock.
do_execve_common does:
prepare_bprm_creds (takes cred_guard_mutex)
check_unsafe_exec (checks nnp to set LSM_UNSAFE_NO_NEW_PRIVS)
prepare_binprm (handles suid escalation, checks nnp separately)
security_bprm_set_creds (checks LSM_UNSAFE_NO_NEW_PRIVS)
exec_binprm
load_elf_binary
flush_old_exec
de_thread (takes and releases sighand->lock)
install_exec_creds (releases cred_guard_mutex)
I don't see a way to use cred_guard_mutex during tsync (which holds
sighand->lock) without dead-locking. What were you considering here?
Grab cred_guard_mutex and then sighand->lock, perhaps?
On Wed, Jun 25, 2014 at 11:09 AM, Andy Lutomirski [off-list ref] wrote:
On Wed, Jun 25, 2014 at 10:57 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:24 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Kees Cook wrote:
quoted
On Wed, Jun 25, 2014 at 9:52 AM, Oleg Nesterov [off-list ref] wrote:
quoted
Yes, at least this should close the race with suid-exec. And there are no
other users. Except apparmor, and I hope you will check it because I simply
do not know what it does ;)
quoted
I wonder if changes to nnp need to "flushed" during syscall entry
instead of getting updated externally/asynchronously? That way it
won't be out of sync with the seccomp mode/filters.
Perhaps secure computing needs to check some (maybe seccomp-only)
atomic flags and flip on the "real" nnp if found?
Not sure I understand you, could you clarify?
Instead of having TSYNC change the nnp bit, it can set a new flag, say:
task->seccomp.flags |= SECCOMP_NEEDS_NNP;
This would be set along with seccomp.mode, seccomp.filter, and
TIF_SECCOMP. Then, during the next secure_computing() call that thread
makes, it would check the flag:
if (task->seccomp.flags & SECCOMP_NEEDS_NNP)
task->nnp = 1;
This means that nnp couldn't change in the middle of a running syscall.
Aha, so you were worried about the same thing. Not sure we need this,
but at least I understand you and...
quoted
Hmmm. Perhaps this doesn't solve anything, though? Perhaps my proposal
above would actually make things worse, since now we'd have a thread
with seccomp set up, and no nnp. If it was in the middle of exec,
we're still causing a problem.
Yes ;)
quoted
I think we'd also need a way to either delay the seccomp changes, or
to notice this condition during exec. Bleh.
Hmm. confused again,
I mean to suggest that the tsync changes would be stored in each
thread, but somewhere other than the true seccomp struct, but with
TIF_SECCOMP set. When entering secure_computing(), current would check
for the "changes to sync", and apply them, then start the syscall. In
this way, we can never race a syscall (like exec).
I'm not sure that helps. If you set a pending filter part-way through
exec, and exec copies that pending filter but doesn't notice NNP, then
there's an exploitable race.
quoted
quoted
quoted
What actually happens with a multi-threaded process calls exec? I
assume all the other threads are destroyed?
Yes. But this is the point-of-no-return, de_thread() is called after the execing
thared has already passed (say) check_unsafe_exec().
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
I can't tell yet. I'm still trying to understand the order of
operations here. It looks like de_thread() takes the sighand lock.
do_execve_common does:
prepare_bprm_creds (takes cred_guard_mutex)
check_unsafe_exec (checks nnp to set LSM_UNSAFE_NO_NEW_PRIVS)
prepare_binprm (handles suid escalation, checks nnp separately)
security_bprm_set_creds (checks LSM_UNSAFE_NO_NEW_PRIVS)
exec_binprm
load_elf_binary
flush_old_exec
de_thread (takes and releases sighand->lock)
install_exec_creds (releases cred_guard_mutex)
I don't see a way to use cred_guard_mutex during tsync (which holds
sighand->lock) without dead-locking. What were you considering here?
Grab cred_guard_mutex and then sighand->lock, perhaps?
Ah, yes, task->signal is like sighand: shared across all threads.
--
Kees Cook
Chrome OS Security
On Wed, Jun 25, 2014 at 10:24 AM, Oleg Nesterov [off-list ref] wrote:
quoted
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
I can't tell yet. I'm still trying to understand the order of
operations here. It looks like de_thread() takes the sighand lock.
do_execve_common does:
prepare_bprm_creds (takes cred_guard_mutex)
check_unsafe_exec (checks nnp to set LSM_UNSAFE_NO_NEW_PRIVS)
prepare_binprm (handles suid escalation, checks nnp separately)
security_bprm_set_creds (checks LSM_UNSAFE_NO_NEW_PRIVS)
exec_binprm
load_elf_binary
flush_old_exec
de_thread (takes and releases sighand->lock)
install_exec_creds (releases cred_guard_mutex)
Yes, and note that when cred_guard_mutex is dropped all other threads
are already killed,
I don't see a way to use cred_guard_mutex during tsync (which holds
sighand->lock) without dead-locking. What were you considering here?
Just take/drop current->signal->cred_guard_mutex along with ->siglock
in seccomp_set_mode_filter() ? Unconditionally on depending on
SECCOMP_FILTER_FLAG_TSYNC.
Oleg.
On Wed, Jun 25, 2014 at 11:20 AM, Oleg Nesterov [off-list ref] wrote:
On 06/25, Kees Cook wrote:
quoted
On Wed, Jun 25, 2014 at 10:24 AM, Oleg Nesterov [off-list ref] wrote:
quoted
However, do_execve() takes cred_guard_mutex at the start in prepare_bprm_creds()
and drops it in install_exec_creds(), so it should solve the problem?
I can't tell yet. I'm still trying to understand the order of
operations here. It looks like de_thread() takes the sighand lock.
do_execve_common does:
prepare_bprm_creds (takes cred_guard_mutex)
check_unsafe_exec (checks nnp to set LSM_UNSAFE_NO_NEW_PRIVS)
prepare_binprm (handles suid escalation, checks nnp separately)
security_bprm_set_creds (checks LSM_UNSAFE_NO_NEW_PRIVS)
exec_binprm
load_elf_binary
flush_old_exec
de_thread (takes and releases sighand->lock)
install_exec_creds (releases cred_guard_mutex)
Yes, and note that when cred_guard_mutex is dropped all other threads
are already killed,
quoted
I don't see a way to use cred_guard_mutex during tsync (which holds
sighand->lock) without dead-locking. What were you considering here?
Just take/drop current->signal->cred_guard_mutex along with ->siglock
in seccomp_set_mode_filter() ? Unconditionally on depending on
SECCOMP_FILTER_FLAG_TSYNC.
Yeah, this looks good. *whew* Testing it now, so far so good.
Thanks!
-Kees
--
Kees Cook
Chrome OS Security
Extracts the common check/assign logic, and separates the two mode
setting paths to make things more readable with fewer #ifdefs within
function bodies.
Signed-off-by: Kees Cook <redacted>
---
kernel/seccomp.c | 123 +++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 84 insertions(+), 39 deletions(-)
@@ -501,67 +523,83 @@ long prctl_get_seccomp(void)}/**-*seccomp_set_mode:internalfunctionforsettingseccompmode-*@seccomp_mode:requestedmodetouse-*@filter:optionalstructsock_fprogforusewithSECCOMP_MODE_FILTER+*seccomp_set_mode_strict:internalfunctionforsettingstrictseccomp*-*Thisfunctionmaybecalledrepeatedlywitha@seccomp_modeof-*SECCOMP_MODE_FILTERtoinstalladditionalfilters.Everyfilter-*successfullyinstalledwillbeevaluated(inreverseorder)foreachsystem-*callthetaskmakes.+*Oncecurrent->seccomp.modeisnon-zero,itmaynotbechanged.+*+*Returns0onsuccessor-EINVALonfailure.+*/+staticlongseccomp_set_mode_strict(void)+{+constunsignedlongseccomp_mode=SECCOMP_MODE_STRICT;+unsignedlongirqflags;+intret=-EINVAL;++spin_lock_irqsave(¤t->sighand->siglock,irqflags);++if(!seccomp_check_mode(current,seccomp_mode))+gotoout;++#ifdef TIF_NOTSC+disable_TSC();+#endif+seccomp_assign_mode(current,seccomp_mode);+ret=0;++out:+spin_unlock_irqrestore(¤t->sighand->siglock,irqflags);++returnret;+}++#ifdef CONFIG_SECCOMP_FILTER+/**+*seccomp_set_mode_filter:internalfunctionforsettingseccompfilter+*@filter:structsock_fprogcontainingfilter+*+*Thisfunctionmaybecalledrepeatedlytoinstalladditionalfilters.+*Everyfiltersuccessfullyinstalledwillbeevaluated(inreverseorder)+*foreachsystemcallthetaskmakes.**Oncecurrent->seccomp.modeisnon-zero,itmaynotbechanged.**Returns0onsuccessor-EINVALonfailure.*/-staticlongseccomp_set_mode(unsignedlongseccomp_mode,char__user*filter)+staticlongseccomp_set_mode_filter(char__user*filter){+constunsignedlongseccomp_mode=SECCOMP_MODE_FILTER;structseccomp_filter*prepared=NULL;unsignedlongirqflags;longret=-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))-returnPTR_ERR(prepared);-}-#endif+/* Prepare the new filter outside of any locking. */+prepared=seccomp_prepare_user_filter(filter);+if(IS_ERR(prepared))+returnPTR_ERR(prepared);spin_lock_irqsave(¤t->sighand->siglock,irqflags);-if(current->seccomp.mode&&-current->seccomp.mode!=seccomp_mode)+if(!seccomp_check_mode(current,seccomp_mode))gotoout;-switch(seccomp_mode){-caseSECCOMP_MODE_STRICT:-ret=0;-#ifdef TIF_NOTSC-disable_TSC();-#endif-break;-#ifdef CONFIG_SECCOMP_FILTER-caseSECCOMP_MODE_FILTER:-ret=seccomp_attach_filter(prepared);-if(ret)-gotoout;-/* Do not free the successfully attached filter. */-prepared=NULL;-break;-#endif-default:+ret=seccomp_attach_filter(prepared);+if(ret)gotoout;-}+/* Do not free the successfully attached filter. */+prepared=NULL;-current->seccomp.mode=seccomp_mode;-set_thread_flag(TIF_SECCOMP);+seccomp_assign_mode(current,seccomp_mode);out:spin_unlock_irqrestore(¤t->sighand->siglock,irqflags);seccomp_filter_free(prepared);returnret;}+#else+staticinlinelongseccomp_set_mode_filter(char__user*filter)+{+return-EINVAL;+}+#endif/***prctl_set_seccomp:configurescurrent->seccomp.mode
OK, but unless task == current this can race with secure_computing().
I think this needs smp_mb__before_atomic() and secure_computing() needs
rmb() after test_bit(TIF_SECCOMP).
Otherwise, can't __secure_computing() hit BUG() if it sees the old
mode == SECCOMP_MODE_DISABLED ?
Or seccomp_run_filters() can see ->filters == NULL and WARN(),
smp_load_acquire() only serializes that LOAD with the subsequent memory
operations.
Oleg.
OK, but unless task == current this can race with secure_computing().
I think this needs smp_mb__before_atomic() and secure_computing() needs
rmb() after test_bit(TIF_SECCOMP).
Otherwise, can't __secure_computing() hit BUG() if it sees the old
mode == SECCOMP_MODE_DISABLED ?
Or seccomp_run_filters() can see ->filters == NULL and WARN(),
smp_load_acquire() only serializes that LOAD with the subsequent memory
operations.
Hm, actually, now I'm worried about smp_load_acquire() being too slow
in run_filters().
The ordering must be:
- task->seccomp.filter must be valid before
- task->seccomp.mode is set, which must be valid before
- TIF_SECCOMP is set
But I don't want to impact secure_computing(). What's the best way to
make sure this ordering is respected?
-Kees
--
Kees Cook
Chrome OS Security
OK, but unless task == current this can race with secure_computing().
I think this needs smp_mb__before_atomic() and secure_computing() needs
rmb() after test_bit(TIF_SECCOMP).
Otherwise, can't __secure_computing() hit BUG() if it sees the old
mode == SECCOMP_MODE_DISABLED ?
Or seccomp_run_filters() can see ->filters == NULL and WARN(),
smp_load_acquire() only serializes that LOAD with the subsequent memory
operations.
Hm, actually, now I'm worried about smp_load_acquire() being too slow
in run_filters().
The ordering must be:
- task->seccomp.filter must be valid before
- task->seccomp.mode is set, which must be valid before
- TIF_SECCOMP is set
But I don't want to impact secure_computing(). What's the best way to
make sure this ordering is respected?
Remove the ordering requirement, perhaps?
What if you moved mode into seccomp.filter? Then there would be
little reason to check TIF_SECCOMP from secure_computing; instead, you
could smp_load_acquire (or read_barrier_depends, maybe) seccomp.filter
from secure_computing and pass the result as a parameter to
__secure_computing. Or you could even remove the distinction between
secure_computing and __secure_computing -- it's essentially useless
anyway to split entry hook approaches like my x86 fastpath prototype.
--Andy
OK, but unless task == current this can race with secure_computing().
I think this needs smp_mb__before_atomic() and secure_computing() needs
rmb() after test_bit(TIF_SECCOMP).
Otherwise, can't __secure_computing() hit BUG() if it sees the old
mode == SECCOMP_MODE_DISABLED ?
Or seccomp_run_filters() can see ->filters == NULL and WARN(),
smp_load_acquire() only serializes that LOAD with the subsequent memory
operations.
Hm, actually, now I'm worried about smp_load_acquire() being too slow
in run_filters().
The ordering must be:
- task->seccomp.filter must be valid before
- task->seccomp.mode is set, which must be valid before
- TIF_SECCOMP is set
But I don't want to impact secure_computing(). What's the best way to
make sure this ordering is respected?
Remove the ordering requirement, perhaps?
What if you moved mode into seccomp.filter? Then there would be
little reason to check TIF_SECCOMP from secure_computing; instead, you
could smp_load_acquire (or read_barrier_depends, maybe) seccomp.filter
from secure_computing and pass the result as a parameter to
__secure_computing. Or you could even remove the distinction between
secure_computing and __secure_computing -- it's essentially useless
anyway to split entry hook approaches like my x86 fastpath prototype.
The TIF_SECCOMP is needed for the syscall entry path. The check in
secure_computing() is just because the "I am being traced" trigger
includes a call to secure_computing, which filters out tracing
reasons.
Your fast path work would clean a lot of that up, as you say. But it
still doesn't change the ordering check here. TIF_SECCOMP indicates
seccomp.mode must be checked, so that ordering will remain, and if
mode == FILTER, seccomp.filter must be valid.
Isn't there a way we can force the assignment ordering in seccomp_assign_mode()?
-Kees
--
Kees Cook
Chrome OS Security
OK, but unless task == current this can race with secure_computing().
I think this needs smp_mb__before_atomic() and secure_computing() needs
rmb() after test_bit(TIF_SECCOMP).
Otherwise, can't __secure_computing() hit BUG() if it sees the old
mode == SECCOMP_MODE_DISABLED ?
Or seccomp_run_filters() can see ->filters == NULL and WARN(),
smp_load_acquire() only serializes that LOAD with the subsequent memory
operations.
Hm, actually, now I'm worried about smp_load_acquire() being too slow
in run_filters().
The ordering must be:
- task->seccomp.filter must be valid before
- task->seccomp.mode is set, which must be valid before
- TIF_SECCOMP is set
But I don't want to impact secure_computing(). What's the best way to
make sure this ordering is respected?
Remove the ordering requirement, perhaps?
What if you moved mode into seccomp.filter? Then there would be
little reason to check TIF_SECCOMP from secure_computing; instead, you
could smp_load_acquire (or read_barrier_depends, maybe) seccomp.filter
from secure_computing and pass the result as a parameter to
__secure_computing. Or you could even remove the distinction between
secure_computing and __secure_computing -- it's essentially useless
anyway to split entry hook approaches like my x86 fastpath prototype.
The TIF_SECCOMP is needed for the syscall entry path. The check in
secure_computing() is just because the "I am being traced" trigger
includes a call to secure_computing, which filters out tracing
reasons.
Right. I'm suggesting just checking a single indication that seccomp
is on from the process in the seccomp code so that the order doesn't
matter.
IOW, TIF_SECCOMP causes __secure_computing to be invoked, but the race
only seems to matter because of the warning and the BUG. I think that
both can be fixed if you merge mode into filter so that
__secure_computing atomically checks one condition.
Your fast path work would clean a lot of that up, as you say. But it
still doesn't change the ordering check here. TIF_SECCOMP indicates
seccomp.mode must be checked, so that ordering will remain, and if
mode == FILTER, seccomp.filter must be valid.
Isn't there a way we can force the assignment ordering in seccomp_assign_mode()?
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
--Andy
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Oleg.
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-25 17:38:41
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
--Andy
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Oleg.
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
--
Kees Cook
Chrome OS Security
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-25 18:08:17
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
--Andy
On Wed, Jun 25, 2014 at 11:07 AM, Andy Lutomirski [off-list ref] wrote:
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
It looks like SMP ARM issues dsb for rmb, which seems a bit expensive.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CIHJFGFE.html
If I skip the rmb in the secure_computing call before checking mode,
it sounds like I run the risk of racing an out-of-order TIF_SECCOMP vs
mode and filter. This seems unlikely to me, given an addition of the
smp_mb__before_atomic() during the seccomp_assign_mode()? I guess I
don't have a sense of how aggressively ARM might do data caching in
this area. Could the other thread actually see TIF_SECCOMP get set but
still have an out of date copy of seccomp.mode?
I really want to avoid adding anything to the secure_computing()
execution path. :(
-Kees
--
Kees Cook
Chrome OS Security
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-27 18:39:50
On Fri, Jun 27, 2014 at 11:33 AM, Kees Cook [off-list ref] wrote:
On Wed, Jun 25, 2014 at 11:07 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
It looks like SMP ARM issues dsb for rmb, which seems a bit expensive.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CIHJFGFE.html
If I skip the rmb in the secure_computing call before checking mode,
it sounds like I run the risk of racing an out-of-order TIF_SECCOMP vs
mode and filter. This seems unlikely to me, given an addition of the
smp_mb__before_atomic() during the seccomp_assign_mode()? I guess I
don't have a sense of how aggressively ARM might do data caching in
this area. Could the other thread actually see TIF_SECCOMP get set but
still have an out of date copy of seccomp.mode?
I really want to avoid adding anything to the secure_computing()
execution path. :(
Hence my suggestion to make the ordering not matter. No ordering
requirement, no barriers.
--Andy
On Fri, Jun 27, 2014 at 11:39 AM, Andy Lutomirski [off-list ref] wrote:
On Fri, Jun 27, 2014 at 11:33 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:07 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
It looks like SMP ARM issues dsb for rmb, which seems a bit expensive.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CIHJFGFE.html
If I skip the rmb in the secure_computing call before checking mode,
it sounds like I run the risk of racing an out-of-order TIF_SECCOMP vs
mode and filter. This seems unlikely to me, given an addition of the
smp_mb__before_atomic() during the seccomp_assign_mode()? I guess I
don't have a sense of how aggressively ARM might do data caching in
this area. Could the other thread actually see TIF_SECCOMP get set but
still have an out of date copy of seccomp.mode?
I really want to avoid adding anything to the secure_computing()
execution path. :(
Hence my suggestion to make the ordering not matter. No ordering
requirement, no barriers.
I may be misunderstanding something, but I think there's still an
ordering problem. We'll have TIF_SECCOMP already, so if we enter
secure_computing with a NULL filter, we'll kill the process.
Merging .mode and .filter would remove one of the race failure paths:
having TIF_SECCOMP and not having a mode set (leading to BUG). With
the merge, we could still race and land in the same place as have
TIF_SECCOMP and mode==2, but filter==NULL, leading to WARN and kill.
I guess the question is how large is the race risk on ARM? Is it
possible to have TIF_SECCOMP that far out of sync for the thread?
-Kees
--
Kees Cook
Chrome OS Security
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-27 18:56:59
On Fri, Jun 27, 2014 at 11:52 AM, Kees Cook [off-list ref] wrote:
On Fri, Jun 27, 2014 at 11:39 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Fri, Jun 27, 2014 at 11:33 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:07 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
It looks like SMP ARM issues dsb for rmb, which seems a bit expensive.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CIHJFGFE.html
If I skip the rmb in the secure_computing call before checking mode,
it sounds like I run the risk of racing an out-of-order TIF_SECCOMP vs
mode and filter. This seems unlikely to me, given an addition of the
smp_mb__before_atomic() during the seccomp_assign_mode()? I guess I
don't have a sense of how aggressively ARM might do data caching in
this area. Could the other thread actually see TIF_SECCOMP get set but
still have an out of date copy of seccomp.mode?
I really want to avoid adding anything to the secure_computing()
execution path. :(
Hence my suggestion to make the ordering not matter. No ordering
requirement, no barriers.
I may be misunderstanding something, but I think there's still an
ordering problem. We'll have TIF_SECCOMP already, so if we enter
secure_computing with a NULL filter, we'll kill the process.
Merging .mode and .filter would remove one of the race failure paths:
having TIF_SECCOMP and not having a mode set (leading to BUG). With
the merge, we could still race and land in the same place as have
TIF_SECCOMP and mode==2, but filter==NULL, leading to WARN and kill.
You could just make secure_computing do nothing if filter == NULL.
It's probably faster to test that than TIF_SECCOMP anyway, since you
need to read the filter cacheline regardless, and testing a regular
variable for non-NULLness might be faster than an atomic bit test
operation. (Or may not -- I don't know.)
I guess the question is how large is the race risk on ARM? Is it
possible to have TIF_SECCOMP that far out of sync for the thread?
Dunno. I don't like leaving crashy known races around.
--Andy
On Fri, Jun 27, 2014 at 11:56 AM, Andy Lutomirski [off-list ref] wrote:
On Fri, Jun 27, 2014 at 11:52 AM, Kees Cook [off-list ref] wrote:
quoted
On Fri, Jun 27, 2014 at 11:39 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Fri, Jun 27, 2014 at 11:33 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:07 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
It looks like SMP ARM issues dsb for rmb, which seems a bit expensive.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CIHJFGFE.html
If I skip the rmb in the secure_computing call before checking mode,
it sounds like I run the risk of racing an out-of-order TIF_SECCOMP vs
mode and filter. This seems unlikely to me, given an addition of the
smp_mb__before_atomic() during the seccomp_assign_mode()? I guess I
don't have a sense of how aggressively ARM might do data caching in
this area. Could the other thread actually see TIF_SECCOMP get set but
still have an out of date copy of seccomp.mode?
I really want to avoid adding anything to the secure_computing()
execution path. :(
Hence my suggestion to make the ordering not matter. No ordering
requirement, no barriers.
I may be misunderstanding something, but I think there's still an
ordering problem. We'll have TIF_SECCOMP already, so if we enter
secure_computing with a NULL filter, we'll kill the process.
Merging .mode and .filter would remove one of the race failure paths:
having TIF_SECCOMP and not having a mode set (leading to BUG). With
the merge, we could still race and land in the same place as have
TIF_SECCOMP and mode==2, but filter==NULL, leading to WARN and kill.
You could just make secure_computing do nothing if filter == NULL.
It's probably faster to test that than TIF_SECCOMP anyway, since you
need to read the filter cacheline regardless, and testing a regular
variable for non-NULLness might be faster than an atomic bit test
operation. (Or may not -- I don't know.)
I am uncomfortable about making filter == NULL be a "fail open"
condition if TIF_SECCOMP is set.
quoted
I guess the question is how large is the race risk on ARM? Is it
possible to have TIF_SECCOMP that far out of sync for the thread?
Dunno. I don't like leaving crashy known races around.
Yeah, me too. Hrmpf. I will do some rmb() timing tests...
-Kees
--
Kees Cook
Chrome OS Security
From: Andy Lutomirski <luto@amacapital.net> Date: 2014-06-27 19:11:36
On Fri, Jun 27, 2014 at 12:04 PM, Kees Cook [off-list ref] wrote:
On Fri, Jun 27, 2014 at 11:56 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Fri, Jun 27, 2014 at 11:52 AM, Kees Cook [off-list ref] wrote:
quoted
On Fri, Jun 27, 2014 at 11:39 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Fri, Jun 27, 2014 at 11:33 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:07 AM, Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 11:00 AM, Kees Cook [off-list ref] wrote:
quoted
On Wed, Jun 25, 2014 at 10:51 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
On Wed, Jun 25, 2014 at 10:32 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 06/25, Andy Lutomirski wrote:
quoted
Write the filter, then smp_mb (or maybe a weaker barrier is okay),
then set the bit.
Yes, exactly, this is what I meant. Plas rmb() in __secure_computing().
But I still can't understand the rest of your discussion about the
ordering we need ;)
Let me try again from scratch.
Currently there are three relevant variables: TIF_SECCOMP,
seccomp.mode, and seccomp.filter. __secure_computing needs
seccomp.mode and seccomp.filter to be in sync, and it wants (but
doesn't really need) TIF_SECCOMP to be in sync as well.
My suggestion is to rearrange it a bit. Move mode into seccomp.filter
(so that filter == NULL implies no seccomp) and don't check
This would require that we reimplement mode 1 seccomp via mode 2
filters. Which isn't too hard, but may add complexity.
quoted
quoted
TIF_SECCOMP in secure_computing. Then turning on seccomp is entirely
atomic except for the fact that the seccomp hooks won't be called if
filter != NULL but !TIF_SECCOMP. This removes all ordering
requirements.
Ah, got it, thanks. Perhaps I missed somehing, but to me this looks like
unnecessary complication at first glance.
We alredy have TIF_SECCOMP, we need it anyway, and we should only care
about the case when this bit is actually set, so that we can race with
the 1st call of __secure_computing().
Otherwise we are fine: we can miss the new filter anyway, ->mode can't
be changed it is already nonzero.
quoted
Alternatively, __secure_computing could still BUG_ON(!seccomp.filter).
In that case, filter needs to be set before TIF_SECCOMP is set, but
that's straightforward.
Yep. And this is how seccomp_assign_mode() already works? It is called
after we change ->filter chain, it changes ->mode before set(TIF_SECCOMP)
just it lacks a barrier.
Right, I think the best solution is to add the barrier. I was
concerned that adding the read barrier in secure_computing would have
a performance impact, though.
I can't speak for ARM, but I think that all of the read barriers are
essentially free on x86. (smp_mb is a very different story, but that
shouldn't be needed here.)
It looks like SMP ARM issues dsb for rmb, which seems a bit expensive.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CIHJFGFE.html
If I skip the rmb in the secure_computing call before checking mode,
it sounds like I run the risk of racing an out-of-order TIF_SECCOMP vs
mode and filter. This seems unlikely to me, given an addition of the
smp_mb__before_atomic() during the seccomp_assign_mode()? I guess I
don't have a sense of how aggressively ARM might do data caching in
this area. Could the other thread actually see TIF_SECCOMP get set but
still have an out of date copy of seccomp.mode?
I really want to avoid adding anything to the secure_computing()
execution path. :(
Hence my suggestion to make the ordering not matter. No ordering
requirement, no barriers.
I may be misunderstanding something, but I think there's still an
ordering problem. We'll have TIF_SECCOMP already, so if we enter
secure_computing with a NULL filter, we'll kill the process.
Merging .mode and .filter would remove one of the race failure paths:
having TIF_SECCOMP and not having a mode set (leading to BUG). With
the merge, we could still race and land in the same place as have
TIF_SECCOMP and mode==2, but filter==NULL, leading to WARN and kill.
You could just make secure_computing do nothing if filter == NULL.
It's probably faster to test that than TIF_SECCOMP anyway, since you
need to read the filter cacheline regardless, and testing a regular
variable for non-NULLness might be faster than an atomic bit test
operation. (Or may not -- I don't know.)
I am uncomfortable about making filter == NULL be a "fail open"
condition if TIF_SECCOMP is set.
I'm unconvinced here. TIF_SECCOMP unset is already a fail-open
condition if filter is set.
--Andy
I must have missed something but I do not understand your concerns.
__secure_computing() is not trivial, and we are going to execute the
filters. Do you really think rmb() can add the noticeable difference?
Not to mention that we can only get here if we take the slow syscall
enter path due to TIF_SECCOMP...
Oleg.
I must have missed something but I do not understand your concerns.
__secure_computing() is not trivial, and we are going to execute the
filters. Do you really think rmb() can add the noticeable difference?
Not to mention that we can only get here if we take the slow syscall
enter path due to TIF_SECCOMP...
On my box, with my fancy multi-phase seccomp patches, the total
seccomp overhead for a very short filter is about 13ns. Adding a full
barrier would add several ns, I think.
Admittedly, this is x86, not ARM, so comparisons here are completely
bogus. And that read memory barrier doesn't even need an instruction
on x86. But still, let's try to keep this fast.
--Andy
I must have missed something but I do not understand your concerns.
__secure_computing() is not trivial, and we are going to execute the
filters. Do you really think rmb() can add the noticeable difference?
Not to mention that we can only get here if we take the slow syscall
enter path due to TIF_SECCOMP...
On my box, with my fancy multi-phase seccomp patches, the total
seccomp overhead for a very short filter is about 13ns. Adding a full
barrier would add several ns, I think.
I am just curious, does this 13ns overhead include the penalty from the
slow syscall enter path triggered by TIF_SECCOMP ?
Admittedly, this is x86, not ARM, so comparisons here are completely
bogus. And that read memory barrier doesn't even need an instruction
on x86. But still, let's try to keep this fast.
Well, I am not going to insist...
But imo it would be better to make it correct in a most simple way, then
we can optimize this code and see if there is a noticeable difference.
Not only we can change the ordering, we can remove the BUG_ON's and just
accept the fact the __secure_computing() can race with sys_seccomp() from
another thread.
If nothing else, it would be much simpler to discuss this patch if it comes
as a separate change.
Oleg.
I must have missed something but I do not understand your concerns.
__secure_computing() is not trivial, and we are going to execute the
filters. Do you really think rmb() can add the noticeable difference?
Not to mention that we can only get here if we take the slow syscall
enter path due to TIF_SECCOMP...
On my box, with my fancy multi-phase seccomp patches, the total
seccomp overhead for a very short filter is about 13ns. Adding a full
barrier would add several ns, I think.
I am just curious, does this 13ns overhead include the penalty from the
slow syscall enter path triggered by TIF_SECCOMP ?
Yes, which is more or less the whole point of that patch series. I
rewrote part of the TIF_SECCOMP-but-no-tracing case in assembly :)
I'm playing with rewriting it in C, but it's looking like it'll be a
bit more far-reaching than I was hoping.
--Andy
I must have missed something but I do not understand your concerns.
__secure_computing() is not trivial, and we are going to execute the
filters. Do you really think rmb() can add the noticeable difference?
Not to mention that we can only get here if we take the slow syscall
enter path due to TIF_SECCOMP...
On my box, with my fancy multi-phase seccomp patches, the total
seccomp overhead for a very short filter is about 13ns. Adding a full
barrier would add several ns, I think.
I am just curious, does this 13ns overhead include the penalty from the
slow syscall enter path triggered by TIF_SECCOMP ?
quoted
Admittedly, this is x86, not ARM, so comparisons here are completely
bogus. And that read memory barrier doesn't even need an instruction
on x86. But still, let's try to keep this fast.
Well, I am not going to insist...
But imo it would be better to make it correct in a most simple way, then
we can optimize this code and see if there is a noticeable difference.
Not only we can change the ordering, we can remove the BUG_ON's and just
accept the fact the __secure_computing() can race with sys_seccomp() from
another thread.
If nothing else, it would be much simpler to discuss this patch if it comes
as a separate change.
Yeah, the way I want to go is to add the rmb() for now (it gets us
"correctness"), and then later deal with any potential performance
problems on ARM at a later time. (At present, I'm unable to perform
any timings on ARM -- maybe next week.)
I will send the v9 series in a moment here...
-Kees
--
Kees Cook
Chrome OS Security
OK, but unless task == current this can race with secure_computing().
I think this needs smp_mb__before_atomic() and secure_computing() needs
rmb() after test_bit(TIF_SECCOMP).
Otherwise, can't __secure_computing() hit BUG() if it sees the old
mode == SECCOMP_MODE_DISABLED ?
Or seccomp_run_filters() can see ->filters == NULL and WARN(),
smp_load_acquire() only serializes that LOAD with the subsequent memory
operations.
Hm, actually, now I'm worried about smp_load_acquire() being too slow
in run_filters().
The ordering must be:
- task->seccomp.filter must be valid before
- task->seccomp.mode is set, which must be valid before
- TIF_SECCOMP is set
But I don't want to impact secure_computing(). What's the best way to
make sure this ordering is respected?
Cough, confused... can't understand even after I read the email from Andy.
We do not care if __secure_computing() misses the recently added filter,
this can happen anyway, whatever we do.
seccomp.mode is frozen after we set it != SECCOMP_MODE_DISABLED.
So we should only worry if set_tsk_thread_flag(TIF_SECCOMP) actually
changes this bit and makes __secure_computing() possible. If we add
smp_mb__before_atomic() into seccomp_assign_mode() and rmb() at the
start of __secure_computing() everything should be fine?
Oleg.
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 unique to 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 tasklist_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>
---
include/linux/seccomp.h | 6 +++---
kernel/fork.c | 34 +++++++++++++++++++++++++++++++++-
kernel/seccomp.c | 18 +++++++++++++-----
3 files changed, 49 insertions(+), 9 deletions(-)
@@ -524,6 +529,8 @@ static long seccomp_set_mode(unsigned long seccomp_mode, char __user *filter) } #endif+ spin_lock_irqsave(¤t->sighand->siglock, irqflags);+
Well, I won't argue if you prefer to use _irqsave "just in case".
But irqs must be enabled in syscall paths, you could use spin_lock_irq().
The same for seccomp_set_mode_filter() added later.
Oleg.
+static void copy_seccomp(struct task_struct *p)
+{
+#ifdef CONFIG_SECCOMP
+ /*
+ * Must be called with sighand->lock held, which is common to
+ * all threads in the group. Regardless, nothing special is
+ * needed for the child since it is not yet in the tasklist.
+ */
+ BUG_ON(!spin_is_locked(¤t->sighand->siglock));
+
+ get_seccomp_filter(current);
+ p->seccomp = current->seccomp;
+
+ if (p->seccomp.mode != SECCOMP_MODE_DISABLED)
+ set_tsk_thread_flag(p, TIF_SECCOMP);
+#endif
+}
Wait. But what about no_new_privs? We should copy it as well...
Perhaps this helper should be updated a bit and moved into seccomp.c so
that seccomp_sync_threads() could use it too.
Oleg.
+static void copy_seccomp(struct task_struct *p)
+{
+#ifdef CONFIG_SECCOMP
+ /*
+ * Must be called with sighand->lock held, which is common to
+ * all threads in the group. Regardless, nothing special is
+ * needed for the child since it is not yet in the tasklist.
+ */
+ BUG_ON(!spin_is_locked(¤t->sighand->siglock));
+
+ get_seccomp_filter(current);
+ p->seccomp = current->seccomp;
+
+ if (p->seccomp.mode != SECCOMP_MODE_DISABLED)
+ set_tsk_thread_flag(p, TIF_SECCOMP);
+#endif
+}
Wait. But what about no_new_privs? We should copy it as well...
Perhaps this helper should be updated a bit and moved into seccomp.c so
that seccomp_sync_threads() could use it too.
This way we can also unexport get_seccomp_filter().
Oleg.
On Wed, Jun 25, 2014 at 11:07 AM, Oleg Nesterov [off-list ref] wrote:
On 06/24, Kees Cook wrote:
quoted
+static void copy_seccomp(struct task_struct *p)
+{
+#ifdef CONFIG_SECCOMP
+ /*
+ * Must be called with sighand->lock held, which is common to
+ * all threads in the group. Regardless, nothing special is
+ * needed for the child since it is not yet in the tasklist.
+ */
+ BUG_ON(!spin_is_locked(¤t->sighand->siglock));
+
+ get_seccomp_filter(current);
+ p->seccomp = current->seccomp;
+
+ if (p->seccomp.mode != SECCOMP_MODE_DISABLED)
+ set_tsk_thread_flag(p, TIF_SECCOMP);
+#endif
+}
Wait. But what about no_new_privs? We should copy it as well...
Perhaps this helper should be updated a bit and moved into seccomp.c so
that seccomp_sync_threads() could use it too.
Ah! Yes. I had been thinking it had been copied during the task_struct
duplication, but that would have been before holding sighand->lock, so
it needs explicit recopying. Thanks!
-Kees
--
Kees Cook
Chrome OS Security
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 in the following patch, 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>
Cc: linux-api@vger.kernel.org
---
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 | 63 ++++++++++++++++++++++++++++++++-----
kernel/sys_ni.c | 3 ++
8 files changed, 70 insertions(+), 9 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
@@ -582,7 +591,7 @@ static long seccomp_set_mode_filter(char __user *filter)if(!seccomp_check_mode(current,seccomp_mode))gotoout;-ret=seccomp_attach_filter(prepared);+ret=seccomp_attach_filter(flags,prepared);if(ret)gotoout;/* Do not free the successfully attached filter. */
@@ -595,12 +604,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
@@ -610,12 +642,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);}
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 <redacted>
---
kernel/seccomp.c | 97 +++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 67 insertions(+), 30 deletions(-)
@@ -196,27 +196,21 @@ static u32 seccomp_run_filters(int syscall)}/**-*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;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);/**Installingaseccompfilterrequiresthatthetaskhas
@@ -227,11 +221,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;+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;
@@ -275,13 +269,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);
@@ -488,8 +512,18 @@ long prctl_get_seccomp(void)*/staticlongseccomp_set_mode(unsignedlongseccomp_mode,char__user*filter){+structseccomp_filter*prepared=NULL;longret=-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))+returnPTR_ERR(prepared);+}+#endif+if(current->seccomp.mode&¤t->seccomp.mode!=seccomp_mode)gotoout;
@@ -503,9 +537,11 @@ static long seccomp_set_mode(unsigned long seccomp_mode, char __user *filter)break;#ifdef CONFIG_SECCOMP_FILTERcaseSECCOMP_MODE_FILTER:-ret=seccomp_attach_user_filter(filter);+ret=seccomp_attach_filter(prepared);if(ret)gotoout;+/* Do not free the successfully attached filter. */+prepared=NULL;break;#endifdefault:
@@ -515,6 +551,7 @@ static long seccomp_set_mode(unsigned long seccomp_mode, char __user *filter)current->seccomp.mode=seccomp_mode;set_thread_flag(TIF_SECCOMP);out:+seccomp_filter_free(prepared);returnret;}
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>
---
fs/exec.c | 4 ++--
include/linux/sched.h | 16 ++++++++++++++--
kernel/seccomp.c | 2 +-
kernel/sys.c | 4 ++--
security/apparmor/domain.c | 4 ++--
5 files changed, 21 insertions(+), 9 deletions(-)
@@ -1307,8 +1307,7 @@ struct task_struct {*execve*/unsignedin_iowait:1;-/* task may not gain privileges */-unsignedno_new_privs:1;+unsignedlongatomic_flags;/* Flags needing atomic access. *//* Revert to default priority/policy when forking */unsignedsched_reset_on_fork:1;
@@ -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*/
@@ -1307,8 +1307,7 @@ struct task_struct {*execve*/unsignedin_iowait:1;-/* task may not gain privileges */-unsignedno_new_privs:1;+unsignedlongatomic_flags;/* Flags needing atomic access. *//* Revert to default priority/policy when forking */unsignedsched_reset_on_fork:1;
Agreed, personally I like it more than seccomp->flags.
But probably it would be better to place the new member before/after
other bitfields to save the space?
Oleg.
@@ -1307,8 +1307,7 @@ struct task_struct {*execve*/unsignedin_iowait:1;-/* task may not gain privileges */-unsignedno_new_privs:1;+unsignedlongatomic_flags;/* Flags needing atomic access. *//* Revert to default priority/policy when forking */unsignedsched_reset_on_fork:1;
Agreed, personally I like it more than seccomp->flags.
But probably it would be better to place the new member before/after
other bitfields to save the space?
Sure, I'll move it down. (Though I thought the compiler was smarter about that.)
-Kees
--
Kees Cook
Chrome OS Security
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>
---
kernel/seccomp.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
Combines documentation from prctl, in-kernel seccomp_filter.txt and
dropper.c, along with details specific to the new syscall.
Signed-off-by: Kees Cook <redacted>
---
v3:
- change args to void * (luto)
- small typo cleanups
v2:
- add full example code, based on "dropper.c" in samples/seccomp/
---
man2/seccomp.2 | 400 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 400 insertions(+)
create mode 100644 man2/seccomp.2
@@ -0,0 +1,400 @@+.\" Copyright (C) 2014 Kees Cook <keescook@chromium.org>+.\" and Copyright (C) 2012 Will Drewry <wad@chromium.org>+.\" and Copyright (C) 2008 Michael Kerrisk <mtk.manpages@gmail.com>+.\"+.\" %%%LICENSE_START(VERBATIM)+.\" Permission is granted to make and distribute verbatim copies of this+.\" manual provided the copyright notice and this permission notice are+.\" preserved on all copies.+.\"+.\" Permission is granted to copy and distribute modified versions of this+.\" manual under the conditions for verbatim copying, provided that the+.\" entire resulting derived work is distributed under the terms of a+.\" permission notice identical to this one.+.\"+.\" Since the Linux kernel and libraries are constantly changing, this+.\" manual page may be incorrect or out-of-date. The author(s) assume no+.\" responsibility for errors or omissions, or for damages resulting from+.\" the use of the information contained herein. The author(s) may not+.\" have taken the same level of care in the production of this manual,+.\" which is licensed free of charge, as they might when working+.\" professionally.+.\"+.\" Formatted or processed versions of this manual, if unaccompanied by+.\" the source, must acknowledge the copyright and authors of this work.+.\" %%%LICENSE_END+.\"+.THSECCOMP22014-06-23"Linux""Linux Programmer's Manual"+.SHNAME+seccomp \-+operate on Secure Computing state of the process+.SHSYNOPSIS+.nf+.B#include<linux/seccomp.h>+.B#include<linux/filter.h>+.B#include<linux/audit.h>+.B#include<linux/signal.h>+.B#include<sys/ptrace.h>++.BI"int seccomp(unsigned int "operation", unsigned int "flags,+.BI" void *"args);+.fi+.SHDESCRIPTION+The+.BRseccomp()+system call operates on the Secure Computing (seccomp) state of the+current process.++Currently, Linux supports the following+.IRoperation+values:+.TP+.BRSECCOMP_SET_MODE_STRICT+Only system calls that the thread is permitted to make are+.BRread(2),+.BRwrite(2),+.BR_exit(2),+and+.BRsigreturn(2).+Other system calls result in the delivery of a+.BRSIGKILL+signal. Strict secure computing mode is useful for number-crunching+applications that may need to execute untrusted byte code, perhaps+obtained by reading from a pipe or socket.++This operation is available only if the kernel is configured with+.BRCONFIG_SECCOMP+enabled.++The value of+.IRflags+must be 0, and+.IRargs+must be NULL.++This operation is functionally identical to calling+.IR"prctl(PR_SET_SECCOMP,\ SECCOMP_MODE_STRICT)".+.TP+.BRSECCOMP_SET_MODE_FILTER+The system calls allowed are defined by a pointer to a Berkeley Packet+Filter (BPF) passed via+.IRargs.+This argument is a pointer to+.IR"struct\ sock_fprog";+it can be designed to filter arbitrary system calls and system call+arguments. If the filter is invalid, the call will fail, returning+.BREACCESS+in+.IRerrno.++If+.BRfork(2),+.BRclone(2),+or+.BRexecve(2)+are allowed by the filter, any child processes will be constrained to+the same filters and system calls as the parent.++Prior to using this operation, the process must call+.IR"prctl(PR_SET_NO_NEW_PRIVS,\ 1)"+or run with+.BRCAP_SYS_ADMIN+privileges in its namespace. If these are not true, the call will fail+and return+.BREACCES+in+.IRerrno.+This requirement ensures that filter programs cannot be applied to child+processes with greater privileges than the process that installed them.++Additionally, if+.BRprctl(2)+or+.BRseccomp(2)+is allowed by the attached filter, additional filters may be layered on+which will increase evaluation time, but allow for further reduction of+the attack surface during execution of a process.++This operation is available only if the kernel is configured with+.BRCONFIG_SECCOMP_FILTER+enabled.++When+.IRflags+are 0, this operation is functionally identical to calling+.IR"prctl(PR_SET_SECCOMP,\ SECCOMP_MODE_FILTER,\ args)".++The recognized+.IRflags+are:+.RS+.TP+.BRSECCOMP_FILTER_FLAG_TSYNC+When adding a new filter, synchronize all other threads of the current+process to the same seccomp filter tree. If any thread cannot do this,+the call will not attach the new seccomp filter, and will fail returning+the first thread ID found that cannot synchronize. Synchronization will+fail if another thread is in+.BRSECCOMP_MODE_STRICT+or if it has attached new seccomp filters to itself, diverging from the+calling thread's filter tree.+.RE+.SHFILTERS+When adding filters via+.BRSECCOMP_SET_MODE_FILTER,+.IRargs+points to a filter program:++.in+4n+.nf+struct sock_fprog {+ unsigned short len; /* Number of BPF instructions */+ struct sock_filter *filter;+};+.fi+.in++Each program must contain one or more BPF instructions:++.in+4n+.nf+struct sock_filter { /* Filter block */+ __u16 code; /* Actual filter code */+ __u8 jt; /* Jump true */+ __u8 jf; /* Jump false */+ __u32 k; /* Generic multiuse field */+};+.fi+.in++When executing the instructions, the BPF program executes over the+syscall information made available via:++.in+4n+.nf+struct seccomp_data {+ int nr; /* system call number */+ __u32 arch; /* AUDIT_ARCH_* value */+ __u64 instruction_pointer; /* CPU instruction pointer */+ __u64 args[6]; /* up to 6 system call arguments */+};+.fi+.in++A seccomp filter may return any of the following values. If multiple+filters exist, the return value for the evaluation of a given system+call will always use the highest precedent value. (For example,+.BRSECCOMP_RET_KILL+will always take precedence.)++In precedence order, they are:+.TP+.BRSECCOMP_RET_KILL+Results in the task exiting immediately without executing the+system call. The exit status of the task (status & 0x7f) will+be+.BRSIGSYS,+not+.BRSIGKILL.+.TP+.BRSECCOMP_RET_TRAP+Results in the kernel sending a+.BRSIGSYS+signal to the triggering task without executing the system call.+.IRsiginfo\->si_call_addr+will show the address of the system call instruction, and+.IRsiginfo\->si_syscall+and+.IRsiginfo\->si_arch+will indicate which syscall was attempted. The program counter will be+as though the syscall happened (i.e. it will not point to the syscall+instruction). The return value register will contain an arch\-dependent+value; if resuming execution, set it to something sensible.+(The architecture dependency is because replacing it with+.BRENOSYS+could overwrite some useful information.)++The+.BRSECCOMP_RET_DATA+portion of the return value will be passed as+.IRsi_errno.++.BRSIGSYS+triggered by seccomp will have a+.IRsi_code+of+.BRSYS_SECCOMP.+.TP+.BRSECCOMP_RET_ERRNO+Results in the lower 16-bits of the return value being passed+to userland as the+.IRerrno+without executing the system call.+.TP+.BRSECCOMP_RET_TRACE+When returned, this value will cause the kernel to attempt to+notify a ptrace()-based tracer prior to executing the system+call. If there is no tracer present,+.BRENOSYS+is returned to userland and the system call is not executed.++A tracer will be notified if it requests+.BRPTRACE_O_TRACESECCOMP+using+.IRptrace(PTRACE_SETOPTIONS).+The tracer will be notified of a+.BRPTRACE_EVENT_SECCOMP+and the+.BRSECCOMP_RET_DATA+portion of the BPF program return value will be available to the tracer+via+.BRPTRACE_GETEVENTMSG.++The tracer can skip the system call by changing the syscall number+to \-1. Alternatively, the tracer can change the system call+requested by changing the system call to a valid syscall number. If+the tracer asks to skip the system call, then the system call will+appear to return the value that the tracer puts in the return value+register.++The seccomp check will not be run again after the tracer is+notified. (This means that seccomp-based sandboxes MUST NOT+allow use of ptrace, even of other sandboxed processes, without+extreme care; ptracers can use this mechanism to escape.)+.TP+.BRSECCOMP_RET_ALLOW+Results in the system call being executed.++If multiple filters exist, the return value for the evaluation of a+given system call will always use the highest precedent value.++Precedence is only determined using the+.BRSECCOMP_RET_ACTION+mask. When multiple filters return values of the same precedence,+only the+.BRSECCOMP_RET_DATA+from the most recently installed filter will be returned.+.SHRETURNVALUE+On success,+.BRseccomp()+returns 0.+On error, if+.BRSECCOMP_FILTER_FLAG_TSYNC+was used, the return value is the thread ID that caused the+synchronization failure. On other errors, \-1 is returned, and+.IRerrno+is set to indicate the cause of the error.+.SHERRORS+.BRseccomp()+can fail for the following reasons:+.TP+.BREACCESS+the caller did not have the+.BRCAP_SYS_ADMIN+capability, or had not set+.IRno_new_privs+before using+.BRSECCOMP_SET_MODE_FILTER.+.TP+.BREFAULT+.IRargs+was required to be a valid address.+.TP+.BREINVAL+.IRoperation+is unknown; or+.IRflags+are invalid for the given+.IRoperation+.TP+.BRESRCH+Another thread caused a failure during thread sync, but its ID could not+be determined.+.SHVERSIONS+This system call first appeared in Linux 3.16.+.\" FIXME Add glibc version+.SHCONFORMINGTO+This system call is a nonstandard Linux extension.+.SHNOTES+.BRseccomp()+provides a superset of the functionality provided by+.IRPR_SET_SECCOMP+of+.BRprctl(2).+(Which does not support+.IRflags.)+.SHEXAMPLE+.nf+#include <errno.h>+#include <stddef.h>+#include <stdio.h>+#include <stdlib.h>+#include <unistd.h>+#include <linux/audit.h>+#include <linux/filter.h>+#include <linux/seccomp.h>+#include <sys/prctl.h>++static int install_filter(int syscall, int arch, int error)+{+ struct sock_filter filter[] = {+ /* Load architecture. */+ BPF_STMT(BPF_LD+BPF_W+BPF_ABS,+ (offsetof(struct seccomp_data, arch))),+ /* Jump forward 4 instructions on architecture mismatch. */+ BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, arch, 0, 4),+ /* Load syscall number. */+ BPF_STMT(BPF_LD+BPF_W+BPF_ABS,+ (offsetof(struct seccomp_data, nr))),+ /* Jump forward 1 instruction on syscall mismatch. */+ BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, syscall, 0, 1),+ /* Matching arch and syscall: return specific errno. */+ BPF_STMT(BPF_RET+BPF_K,+ SECCOMP_RET_ERRNO|(error & SECCOMP_RET_DATA)),+ /* Destination of syscall mismatch: Allow other syscalls. */+ BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ALLOW),+ /* Destination of arch mismatch: Kill process. */+ BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_KILL),+ };+ struct sock_fprog prog = {+ .len = (unsigned short)(sizeof(filter)/sizeof(filter[0])),+ .filter = filter,+ };+ if (seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog)) {+ perror("seccomp");+ return EXIT_FAILURE;+ }+ return EXIT_SUCCESS;+}++int main(int argc, char **argv)+{+ if (argc < 5) {+ fprintf(stderr, "Usage:\\n"+ "refuse <syscall_nr> <arch> <errno> <prog> [<args>]\\n"+ "Hint: AUDIT_ARCH_I386: 0x%X\\n"+ " AUDIT_ARCH_X86_64: 0x%X\\n"+ "\\n", AUDIT_ARCH_I386, AUDIT_ARCH_X86_64);+ return EXIT_FAILURE;+ }+ if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {+ perror("prctl");+ return EXIT_FAILURE;+ }+ if (install_filter(strtol(argv[1], NULL, 0),+ strtol(argv[2], NULL, 0),+ strtol(argv[3], NULL, 0)))+ return EXIT_FAILURE;+ execv(argv[4], &argv[4]);+ perror("execv");+ return EXIT_FAILURE;+}+.fi+.SHSEEALSO+.adl+.nh+.BRprctl(2),+.BRptrace(2),+.BRsignal(7),+.BRsocket(7)+.ad