linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: linux-kernel@vger.kernel.org
Cc: linux-arch@vger.kernel.org, linux-mips@linux-mips.org,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	linux-security-module@vger.kernel.org, linux-api@vger.kernel.org,
	x86@kernel.org, James Morris <jmorris@namei.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Daniel Borkmann <dborkman@redhat.com>,
	Julien Tinnes <jln@chromium.org>,
	"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	David Drysdale <drysdale@google.com>,
	linux-arm-kernel@lists.infradead.org,
	Alexei Starovoitov <ast@plumgrid.com>
Subject: [PATCH v11 10/11] seccomp: allow mode setting across threads
Date: Wed, 16 Jul 2014 14:50:41 -0700	[thread overview]
Message-ID: <1405547442-26641-11-git-send-email-keescook@chromium.org> (raw)
In-Reply-To: <1405547442-26641-1-git-send-email-keescook@chromium.org>

This changes the mode setting helper to allow threads to change the
seccomp mode from another thread. We must maintain barriers to keep
TIF_SECCOMP synchronized with the rest of the seccomp state.

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
 kernel/seccomp.c |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index d5543e787e4e..9065d2c79c56 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -173,21 +173,24 @@ static int seccomp_check_filter(struct sock_filter *filter, unsigned int flen)
  */
 static u32 seccomp_run_filters(int syscall)
 {
-	struct seccomp_filter *f;
+	struct seccomp_filter *f = ACCESS_ONCE(current->seccomp.filter);
 	struct seccomp_data sd;
 	u32 ret = SECCOMP_RET_ALLOW;
 
 	/* Ensure unexpected behavior doesn't result in failing open. */
-	if (WARN_ON(current->seccomp.filter == NULL))
+	if (unlikely(WARN_ON(f == NULL)))
 		return SECCOMP_RET_KILL;
 
+	/* Make sure cross-thread synced filter points somewhere sane. */
+	smp_read_barrier_depends();
+
 	populate_seccomp_data(&sd);
 
 	/*
 	 * All filters in the list are evaluated and the lowest BPF return
 	 * value always takes priority (ignoring the DATA).
 	 */
-	for (f = current->seccomp.filter; f; f = f->prev) {
+	for (; f; f = f->prev) {
 		u32 cur_ret = SK_RUN_FILTER(f->prog, (void *)&sd);
 
 		if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION))
@@ -207,12 +210,18 @@ static inline bool seccomp_may_assign_mode(unsigned long seccomp_mode)
 	return true;
 }
 
-static inline void seccomp_assign_mode(unsigned long seccomp_mode)
+static inline void seccomp_assign_mode(struct task_struct *task,
+				       unsigned long seccomp_mode)
 {
-	BUG_ON(!spin_is_locked(&current->sighand->siglock));
+	BUG_ON(!spin_is_locked(&task->sighand->siglock));
 
-	current->seccomp.mode = seccomp_mode;
-	set_tsk_thread_flag(current, TIF_SECCOMP);
+	task->seccomp.mode = seccomp_mode;
+	/*
+	 * Make sure TIF_SECCOMP cannot be set before the mode (and
+	 * filter) is set.
+	 */
+	smp_mb__before_atomic();
+	set_tsk_thread_flag(task, TIF_SECCOMP);
 }
 
 #ifdef CONFIG_SECCOMP_FILTER
@@ -435,12 +444,17 @@ static int mode1_syscalls_32[] = {
 
 int __secure_computing(int this_syscall)
 {
-	int mode = current->seccomp.mode;
 	int exit_sig = 0;
 	int *syscall;
 	u32 ret;
 
-	switch (mode) {
+	/*
+	 * Make sure that any changes to mode from another thread have
+	 * been seen after TIF_SECCOMP was seen.
+	 */
+	rmb();
+
+	switch (current->seccomp.mode) {
 	case SECCOMP_MODE_STRICT:
 		syscall = mode1_syscalls;
 #ifdef CONFIG_COMPAT
@@ -545,7 +559,7 @@ static long seccomp_set_mode_strict(void)
 #ifdef TIF_NOTSC
 	disable_TSC();
 #endif
-	seccomp_assign_mode(seccomp_mode);
+	seccomp_assign_mode(current, seccomp_mode);
 	ret = 0;
 
 out:
@@ -595,7 +609,7 @@ static long seccomp_set_mode_filter(unsigned int flags,
 	/* Do not free the successfully attached filter. */
 	prepared = NULL;
 
-	seccomp_assign_mode(seccomp_mode);
+	seccomp_assign_mode(current, seccomp_mode);
 out:
 	spin_unlock_irq(&current->sighand->siglock);
 	seccomp_filter_free(prepared);
-- 
1.7.9.5

WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: linux-kernel@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Oleg Nesterov <oleg@redhat.com>, James Morris <jmorris@namei.org>,
	"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>,
	Alexei Starovoitov <ast@plumgrid.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Daniel Borkmann <dborkman@redhat.com>,
	Will Drewry <wad@chromium.org>, Julien Tinnes <jln@chromium.org>,
	David Drysdale <drysdale@google.com>,
	linux-api@vger.kernel.org, x86@kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org,
	linux-arch@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: [PATCH v11 10/11] seccomp: allow mode setting across threads
Date: Wed, 16 Jul 2014 14:50:41 -0700	[thread overview]
Message-ID: <1405547442-26641-11-git-send-email-keescook@chromium.org> (raw)
Message-ID: <20140716215041.cZCAlcCfhI_5yUWGYDWAKcY0e2BmPu8KChcsM82-d7Q@z> (raw)
In-Reply-To: <1405547442-26641-1-git-send-email-keescook@chromium.org>

This changes the mode setting helper to allow threads to change the
seccomp mode from another thread. We must maintain barriers to keep
TIF_SECCOMP synchronized with the rest of the seccomp state.

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Andy Lutomirski <luto@amacapital.net>
---
 kernel/seccomp.c |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index d5543e787e4e..9065d2c79c56 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -173,21 +173,24 @@ static int seccomp_check_filter(struct sock_filter *filter, unsigned int flen)
  */
 static u32 seccomp_run_filters(int syscall)
 {
-	struct seccomp_filter *f;
+	struct seccomp_filter *f = ACCESS_ONCE(current->seccomp.filter);
 	struct seccomp_data sd;
 	u32 ret = SECCOMP_RET_ALLOW;
 
 	/* Ensure unexpected behavior doesn't result in failing open. */
-	if (WARN_ON(current->seccomp.filter == NULL))
+	if (unlikely(WARN_ON(f == NULL)))
 		return SECCOMP_RET_KILL;
 
+	/* Make sure cross-thread synced filter points somewhere sane. */
+	smp_read_barrier_depends();
+
 	populate_seccomp_data(&sd);
 
 	/*
 	 * All filters in the list are evaluated and the lowest BPF return
 	 * value always takes priority (ignoring the DATA).
 	 */
-	for (f = current->seccomp.filter; f; f = f->prev) {
+	for (; f; f = f->prev) {
 		u32 cur_ret = SK_RUN_FILTER(f->prog, (void *)&sd);
 
 		if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION))
@@ -207,12 +210,18 @@ static inline bool seccomp_may_assign_mode(unsigned long seccomp_mode)
 	return true;
 }
 
-static inline void seccomp_assign_mode(unsigned long seccomp_mode)
+static inline void seccomp_assign_mode(struct task_struct *task,
+				       unsigned long seccomp_mode)
 {
-	BUG_ON(!spin_is_locked(&current->sighand->siglock));
+	BUG_ON(!spin_is_locked(&task->sighand->siglock));
 
-	current->seccomp.mode = seccomp_mode;
-	set_tsk_thread_flag(current, TIF_SECCOMP);
+	task->seccomp.mode = seccomp_mode;
+	/*
+	 * Make sure TIF_SECCOMP cannot be set before the mode (and
+	 * filter) is set.
+	 */
+	smp_mb__before_atomic();
+	set_tsk_thread_flag(task, TIF_SECCOMP);
 }
 
 #ifdef CONFIG_SECCOMP_FILTER
@@ -435,12 +444,17 @@ static int mode1_syscalls_32[] = {
 
 int __secure_computing(int this_syscall)
 {
-	int mode = current->seccomp.mode;
 	int exit_sig = 0;
 	int *syscall;
 	u32 ret;
 
-	switch (mode) {
+	/*
+	 * Make sure that any changes to mode from another thread have
+	 * been seen after TIF_SECCOMP was seen.
+	 */
+	rmb();
+
+	switch (current->seccomp.mode) {
 	case SECCOMP_MODE_STRICT:
 		syscall = mode1_syscalls;
 #ifdef CONFIG_COMPAT
@@ -545,7 +559,7 @@ static long seccomp_set_mode_strict(void)
 #ifdef TIF_NOTSC
 	disable_TSC();
 #endif
-	seccomp_assign_mode(seccomp_mode);
+	seccomp_assign_mode(current, seccomp_mode);
 	ret = 0;
 
 out:
@@ -595,7 +609,7 @@ static long seccomp_set_mode_filter(unsigned int flags,
 	/* Do not free the successfully attached filter. */
 	prepared = NULL;
 
-	seccomp_assign_mode(seccomp_mode);
+	seccomp_assign_mode(current, seccomp_mode);
 out:
 	spin_unlock_irq(&current->sighand->siglock);
 	seccomp_filter_free(prepared);
-- 
1.7.9.5


  parent reply	other threads:[~2014-07-16 21:50 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-16 21:50 [PATCH v11 0/11] seccomp: add thread sync ability Kees Cook
2014-07-16 21:50 ` [PATCH v11 01/11] seccomp: create internal mode-setting function Kees Cook
2014-07-16 21:50   ` Kees Cook
2014-07-16 21:50 ` [PATCH v11 03/11] seccomp: split mode setting routines Kees Cook
2014-07-16 21:50   ` Kees Cook
2014-07-16 21:50 ` [PATCH v11 04/11] seccomp: add "seccomp" syscall Kees Cook
2014-07-16 21:50   ` Kees Cook
2014-07-16 21:50 ` [PATCH v11 05/11] ARM: add seccomp syscall Kees Cook
2014-07-16 21:50   ` Kees Cook
2014-07-16 21:50 ` [PATCH v11 06/11] MIPS: " Kees Cook
2014-07-16 21:50   ` Kees Cook
2014-07-16 21:50 ` [PATCH v11 07/11] sched: move no_new_privs into new atomic flags Kees Cook
     [not found] ` <1405547442-26641-1-git-send-email-keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2014-07-16 21:50   ` [PATCH v11 02/11] seccomp: extract check/assign mode helpers Kees Cook
2014-07-16 21:50     ` Kees Cook
2014-07-16 21:50   ` [PATCH v11 08/11] seccomp: split filter prep from check and apply Kees Cook
2014-07-16 21:50     ` Kees Cook
2014-07-16 21:50   ` [PATCH v11 09/11] seccomp: introduce writer locking Kees Cook
2014-07-16 21:50     ` Kees Cook
2014-07-16 21:50 ` Kees Cook [this message]
2014-07-16 21:50   ` [PATCH v11 10/11] seccomp: allow mode setting across threads Kees Cook
2014-07-16 21:50 ` [PATCH v11 11/11] seccomp: implement SECCOMP_FILTER_FLAG_TSYNC Kees Cook
2014-07-17 15:04   ` David Drysdale
2014-07-17 15:04     ` David Drysdale
     [not found]     ` <CAHse=S_32tmusk4ceY4U6GbNpX4PkX12iPPDZFVZ7qgv-RAooA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-07-17 15:45       ` Kees Cook
2014-07-17 15:45         ` Kees Cook
     [not found]         ` <CAGXu5j+dFZdnnK8f-HRrUs2vLeyhWyHh_AY-OynDcp-Ye+dy7Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-07-17 17:52           ` Kees Cook
2014-07-17 17:52             ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405547442-26641-11-git-send-email-keescook@chromium.org \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=ast@plumgrid.com \
    --cc=dborkman@redhat.com \
    --cc=drysdale@google.com \
    --cc=jln@chromium.org \
    --cc=jmorris@namei.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mtk.manpages@gmail.com \
    --cc=oleg@redhat.com \
    --cc=wad@chromium.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).