Re: [PATCH] TIF_NOTSC and SECCOMP prctl

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[Chuck Ebbert]

In-Reply-To: <20060714060932.GE18774@opteron.random>

On Fri, 14 Jul 2006 08:09:32 +0200, andrea@cpushare.com wrote:

> The below patch seems to work, I ported all my client code on top of
> prctl already. (it's a bit more painful to autodetect a kernel with
> CONFIG_SECCOMP turned off but I already adapted to it)

AFAIC the /proc method of controlling seccomp is so ugly it should
just go, but what about backwards compatibility?

I have a couple of questions:


+void disable_TSC(void)
+{
+       if (!test_and_set_thread_flag(TIF_NOTSC))
+               /*
+                * Must flip the CPU state synchronously with
+                * TIF_NOTSC in the current running context.
+                */
+               hard_disable_TSC();
+}

This gets called from sys_prctl().  Do you need to worry about preemption
between the test_and_set and TSC disable?


--- a/include/asm-i386/processor.h      Thu Jul 13 03:03:35 2006 +0700
+++ b/include/asm-i386/processor.h      Fri Jul 14 07:47:57 2006 +0200
@@ -256,6 +256,10 @@ static inline void clear_in_cr4 (unsigne
        cr4 &= ~mask;
        write_cr4(cr4);
 }
+
+extern void hard_disable_TSC(void);
+extern void disable_TSC(void);
+extern void hard_enable_TSC(void);

Maybe these should be inline?  They're really small and that way you
don't need #ifdef around the code for them.


> Reviews are welcome (then I will move into x86-64, all other archs
> supporting seccomp should require no changes despite the API
> change). Thanks.

For x86_64 you need this:

ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/tif-flags-for-debug-regs-and-io-bitmap-in-ctxsw

But I don't think Andi plans on pushing it for 2.6.18.

-- 
Chuck

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[andrea]

On Tue, Jul 18, 2006 at 06:20:20AM -0400, Chuck Ebbert wrote:
> AFAIC the /proc method of controlling seccomp is so ugly it should
> just go, but what about backwards compatibility?

Given that so far CPUShare seems the only user there should be no
problem, I already uploaded a new CPUShare package that handles both
the old and new interfaces transparently, no matter what kernel runs
under it.

> I have a couple of questions:
> 
> 
> +void disable_TSC(void)
> +{
> +       if (!test_and_set_thread_flag(TIF_NOTSC))
> +               /*
> +                * Must flip the CPU state synchronously with
> +                * TIF_NOTSC in the current running context.
> +                */
> +               hard_disable_TSC();
> +}
> 
> This gets called from sys_prctl().  Do you need to worry about preemption
> between the test_and_set and TSC disable?

I tend to completely forget about preempt.

> Maybe these should be inline?  They're really small and that way you
> don't need #ifdef around the code for them.

I wanted to reduce the bytecode overhead to the minimum when seccomp
is set to y, for that I tried to avoided inlines.

> For x86_64 you need this:
> 
> ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/tif-flags-for-debug-regs-and-io-bitmap-in-ctxsw
> 
> But I don't think Andi plans on pushing it for 2.6.18.

Thanks for the pointer.

For now the patch I posted already works on x86-64 and on all other
archs (x86_64 misses the notsc feature for now, but that's not a
problem, the patch is self contained and we can take care of the notsc
for x86-64 later on).

This is the incremental patch to address the preempt=y kernel builds.

diff -r 373f0be00c40 arch/i386/kernel/process.c
--- a/arch/i386/kernel/process.c	Sun Jul 16 15:51:54 2006 +0200
+++ b/arch/i386/kernel/process.c	Tue Jul 18 14:59:23 2006 +0200
@@ -542,12 +542,14 @@ void hard_disable_TSC(void)
 }
 void disable_TSC(void)
 {
+	preempt_disable();
 	if (!test_and_set_thread_flag(TIF_NOTSC))
 		/*
 		 * Must flip the CPU state synchronously with
 		 * TIF_NOTSC in the current running context.
 		 */
 		hard_disable_TSC();
+	preempt_enable();
 }
 void hard_enable_TSC(void)
 {

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[andrea]

Here a repost of the last seccomp patch against current mainline
including the preempt fix. This changes the seccomp API from
/proc/<pid>/seccomp to a prctl (this will produce a smaller kernel)
and it adds a TIF_NOTSC that seccomp sets. Only the current task can
call disable_TSC (obviously because it hasn't a task_t param). This
includes Chuck's patch to give zero runtime cost to the notsc feature.

After applying this patch, seccomp will keep working fine on all other
archs that currently support it too.

Signed-off-by: Andrea Arcangeli <andrea@cpushare.com>

diff -r 93feac10afde -r fa49c58866fe arch/i386/kernel/process.c
--- a/arch/i386/kernel/process.c	Tue Jul 25 11:05:21 2006 -0200
+++ b/arch/i386/kernel/process.c	Tue Jul 25 23:33:52 2006 +0200
@@ -535,8 +535,31 @@ int dump_task_regs(struct task_struct *t
 	return 1;
 }
 
-static noinline void __switch_to_xtra(struct task_struct *next_p,
-				    struct tss_struct *tss)
+#ifdef CONFIG_SECCOMP
+void hard_disable_TSC(void)
+{
+	write_cr4(read_cr4() | X86_CR4_TSD);
+}
+void disable_TSC(void)
+{
+	preempt_disable();
+	if (!test_and_set_thread_flag(TIF_NOTSC))
+		/*
+		 * Must flip the CPU state synchronously with
+		 * TIF_NOTSC in the current running context.
+		 */
+		hard_disable_TSC();
+	preempt_enable();
+}
+void hard_enable_TSC(void)
+{
+	write_cr4(read_cr4() & ~X86_CR4_TSD);
+}
+#endif /* CONFIG_SECCOMP */
+
+static noinline void
+__switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
+		 struct tss_struct *tss)
 {
 	struct thread_struct *next;
 
@@ -552,60 +575,47 @@ static noinline void __switch_to_xtra(st
 		set_debugreg(next->debugreg[7], 7);
 	}
 
-	if (!test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
+#ifdef CONFIG_SECCOMP
+	if (test_tsk_thread_flag(prev_p, TIF_NOTSC) ^
+	    test_tsk_thread_flag(next_p, TIF_NOTSC)) {
+		/* prev and next are different */
+		if (test_tsk_thread_flag(next_p, TIF_NOTSC))
+			hard_disable_TSC();
+		else
+			hard_enable_TSC();
+	}
+#endif
+
+	if (test_tsk_thread_flag(prev_p, TIF_IO_BITMAP) ||
+	    test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
+		if (!test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
+			/*
+			 * Disable the bitmap via an invalid offset. We still cache
+			 * the previous bitmap owner and the IO bitmap contents:
+			 */
+			tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
+			return;
+		}
+
+		if (likely(next == tss->io_bitmap_owner)) {
+			/*
+			 * Previous owner of the bitmap (hence the bitmap content)
+			 * matches the next task, we dont have to do anything but
+			 * to set a valid offset in the TSS:
+			 */
+			tss->io_bitmap_base = IO_BITMAP_OFFSET;
+			return;
+		}
 		/*
-		 * Disable the bitmap via an invalid offset. We still cache
-		 * the previous bitmap owner and the IO bitmap contents:
+		 * Lazy TSS's I/O bitmap copy. We set an invalid offset here
+		 * and we let the task to get a GPF in case an I/O instruction
+		 * is performed.  The handler of the GPF will verify that the
+		 * faulting task has a valid I/O bitmap and, it true, does the
+		 * real copy and restart the instruction.  This will save us
+		 * redundant copies when the currently switched task does not
+		 * perform any I/O during its timeslice.
 		 */
-		tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
-		return;
-	}
-
-	if (likely(next == tss->io_bitmap_owner)) {
-		/*
-		 * Previous owner of the bitmap (hence the bitmap content)
-		 * matches the next task, we dont have to do anything but
-		 * to set a valid offset in the TSS:
-		 */
-		tss->io_bitmap_base = IO_BITMAP_OFFSET;
-		return;
-	}
-	/*
-	 * Lazy TSS's I/O bitmap copy. We set an invalid offset here
-	 * and we let the task to get a GPF in case an I/O instruction
-	 * is performed.  The handler of the GPF will verify that the
-	 * faulting task has a valid I/O bitmap and, it true, does the
-	 * real copy and restart the instruction.  This will save us
-	 * redundant copies when the currently switched task does not
-	 * perform any I/O during its timeslice.
-	 */
-	tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET_LAZY;
-}
-
-/*
- * This function selects if the context switch from prev to next
- * has to tweak the TSC disable bit in the cr4.
- */
-static inline void disable_tsc(struct task_struct *prev_p,
-			       struct task_struct *next_p)
-{
-	struct thread_info *prev, *next;
-
-	/*
-	 * gcc should eliminate the ->thread_info dereference if
-	 * has_secure_computing returns 0 at compile time (SECCOMP=n).
-	 */
-	prev = task_thread_info(prev_p);
-	next = task_thread_info(next_p);
-
-	if (has_secure_computing(prev) || has_secure_computing(next)) {
-		/* slow path here */
-		if (has_secure_computing(prev) &&
-		    !has_secure_computing(next)) {
-			write_cr4(read_cr4() & ~X86_CR4_TSD);
-		} else if (!has_secure_computing(prev) &&
-			   has_secure_computing(next))
-			write_cr4(read_cr4() | X86_CR4_TSD);
+		tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET_LAZY;
 	}
 }
 
@@ -690,11 +700,9 @@ struct task_struct fastcall * __switch_t
 	/*
 	 * Now maybe handle debug registers and/or IO bitmaps
 	 */
-	if (unlikely((task_thread_info(next_p)->flags & _TIF_WORK_CTXSW))
-	    || test_tsk_thread_flag(prev_p, TIF_IO_BITMAP))
-		__switch_to_xtra(next_p, tss);
-
-	disable_tsc(prev_p, next_p);
+	if (unlikely(task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV ||
+		     task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT))
+		__switch_to_xtra(prev_p, next_p, tss);
 
 	return prev_p;
 }
diff -r 93feac10afde -r fa49c58866fe fs/proc/base.c
--- a/fs/proc/base.c	Tue Jul 25 11:05:21 2006 -0200
+++ b/fs/proc/base.c	Tue Jul 25 23:33:52 2006 +0200
@@ -67,7 +67,6 @@
 #include <linux/mount.h>
 #include <linux/security.h>
 #include <linux/ptrace.h>
-#include <linux/seccomp.h>
 #include <linux/cpuset.h>
 #include <linux/audit.h>
 #include <linux/poll.h>
@@ -98,9 +97,6 @@ enum pid_directory_inos {
 	PROC_TGID_TASK,
 	PROC_TGID_STATUS,
 	PROC_TGID_MEM,
-#ifdef CONFIG_SECCOMP
-	PROC_TGID_SECCOMP,
-#endif
 	PROC_TGID_CWD,
 	PROC_TGID_ROOT,
 	PROC_TGID_EXE,
@@ -141,9 +137,6 @@ enum pid_directory_inos {
 	PROC_TID_INO,
 	PROC_TID_STATUS,
 	PROC_TID_MEM,
-#ifdef CONFIG_SECCOMP
-	PROC_TID_SECCOMP,
-#endif
 	PROC_TID_CWD,
 	PROC_TID_ROOT,
 	PROC_TID_EXE,
@@ -212,9 +205,6 @@ static struct pid_entry tgid_base_stuff[
 	E(PROC_TGID_NUMA_MAPS, "numa_maps", S_IFREG|S_IRUGO),
 #endif
 	E(PROC_TGID_MEM,       "mem",     S_IFREG|S_IRUSR|S_IWUSR),
-#ifdef CONFIG_SECCOMP
-	E(PROC_TGID_SECCOMP,   "seccomp", S_IFREG|S_IRUSR|S_IWUSR),
-#endif
 	E(PROC_TGID_CWD,       "cwd",     S_IFLNK|S_IRWXUGO),
 	E(PROC_TGID_ROOT,      "root",    S_IFLNK|S_IRWXUGO),
 	E(PROC_TGID_EXE,       "exe",     S_IFLNK|S_IRWXUGO),
@@ -255,9 +245,6 @@ static struct pid_entry tid_base_stuff[]
 	E(PROC_TID_NUMA_MAPS,  "numa_maps",    S_IFREG|S_IRUGO),
 #endif
 	E(PROC_TID_MEM,        "mem",     S_IFREG|S_IRUSR|S_IWUSR),
-#ifdef CONFIG_SECCOMP
-	E(PROC_TID_SECCOMP,    "seccomp", S_IFREG|S_IRUSR|S_IWUSR),
-#endif
 	E(PROC_TID_CWD,        "cwd",     S_IFLNK|S_IRWXUGO),
 	E(PROC_TID_ROOT,       "root",    S_IFLNK|S_IRWXUGO),
 	E(PROC_TID_EXE,        "exe",     S_IFLNK|S_IRWXUGO),
@@ -991,78 +978,6 @@ static struct file_operations proc_login
 	.write		= proc_loginuid_write,
 };
 #endif
-
-#ifdef CONFIG_SECCOMP
-static ssize_t seccomp_read(struct file *file, char __user *buf,
-			    size_t count, loff_t *ppos)
-{
-	struct task_struct *tsk = get_proc_task(file->f_dentry->d_inode);
-	char __buf[20];
-	loff_t __ppos = *ppos;
-	size_t len;
-
-	if (!tsk)
-		return -ESRCH;
-	/* no need to print the trailing zero, so use only len */
-	len = sprintf(__buf, "%u\n", tsk->seccomp.mode);
-	put_task_struct(tsk);
-	if (__ppos >= len)
-		return 0;
-	if (count > len - __ppos)
-		count = len - __ppos;
-	if (copy_to_user(buf, __buf + __ppos, count))
-		return -EFAULT;
-	*ppos = __ppos + count;
-	return count;
-}
-
-static ssize_t seccomp_write(struct file *file, const char __user *buf,
-			     size_t count, loff_t *ppos)
-{
-	struct task_struct *tsk = get_proc_task(file->f_dentry->d_inode);
-	char __buf[20], *end;
-	unsigned int seccomp_mode;
-	ssize_t result;
-
-	result = -ESRCH;
-	if (!tsk)
-		goto out_no_task;
-
-	/* can set it only once to be even more secure */
-	result = -EPERM;
-	if (unlikely(tsk->seccomp.mode))
-		goto out;
-
-	result = -EFAULT;
-	memset(__buf, 0, sizeof(__buf));
-	count = min(count, sizeof(__buf) - 1);
-	if (copy_from_user(__buf, buf, count))
-		goto out;
-
-	seccomp_mode = simple_strtoul(__buf, &end, 0);
-	if (*end == '\n')
-		end++;
-	result = -EINVAL;
-	if (seccomp_mode && seccomp_mode <= NR_SECCOMP_MODES) {
-		tsk->seccomp.mode = seccomp_mode;
-		set_tsk_thread_flag(tsk, TIF_SECCOMP);
-	} else
-		goto out;
-	result = -EIO;
-	if (unlikely(!(end - __buf)))
-		goto out;
-	result = end - __buf;
-out:
-	put_task_struct(tsk);
-out_no_task:
-	return result;
-}
-
-static struct file_operations proc_seccomp_operations = {
-	.read		= seccomp_read,
-	.write		= seccomp_write,
-};
-#endif /* CONFIG_SECCOMP */
 
 static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
@@ -1753,12 +1668,6 @@ static struct dentry *proc_pident_lookup
 		case PROC_TGID_MEM:
 			inode->i_fop = &proc_mem_operations;
 			break;
-#ifdef CONFIG_SECCOMP
-		case PROC_TID_SECCOMP:
-		case PROC_TGID_SECCOMP:
-			inode->i_fop = &proc_seccomp_operations;
-			break;
-#endif /* CONFIG_SECCOMP */
 		case PROC_TID_MOUNTS:
 		case PROC_TGID_MOUNTS:
 			inode->i_fop = &proc_mounts_operations;
diff -r 93feac10afde -r fa49c58866fe include/asm-i386/processor.h
--- a/include/asm-i386/processor.h	Tue Jul 25 11:05:21 2006 -0200
+++ b/include/asm-i386/processor.h	Tue Jul 25 23:33:52 2006 +0200
@@ -256,6 +256,10 @@ static inline void clear_in_cr4 (unsigne
 	cr4 &= ~mask;
 	write_cr4(cr4);
 }
+
+extern void hard_disable_TSC(void);
+extern void disable_TSC(void);
+extern void hard_enable_TSC(void);
 
 /*
  *      NSC/Cyrix CPU configuration register indexes
diff -r 93feac10afde -r fa49c58866fe include/asm-i386/thread_info.h
--- a/include/asm-i386/thread_info.h	Tue Jul 25 11:05:21 2006 -0200
+++ b/include/asm-i386/thread_info.h	Tue Jul 25 23:33:52 2006 +0200
@@ -142,6 +142,7 @@ static inline struct thread_info *curren
 #define TIF_MEMDIE		16
 #define TIF_DEBUG		17	/* uses debug registers */
 #define TIF_IO_BITMAP		18	/* uses I/O bitmap */
+#define TIF_NOTSC		19	/* TSC is not accessible in userland */
 
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
 #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
@@ -155,6 +156,7 @@ static inline struct thread_info *curren
 #define _TIF_RESTORE_SIGMASK	(1<<TIF_RESTORE_SIGMASK)
 #define _TIF_DEBUG		(1<<TIF_DEBUG)
 #define _TIF_IO_BITMAP		(1<<TIF_IO_BITMAP)
+#define _TIF_NOTSC		(1<<TIF_NOTSC)
 
 /* work to do on interrupt/exception return */
 #define _TIF_WORK_MASK \
@@ -164,7 +166,8 @@ static inline struct thread_info *curren
 #define _TIF_ALLWORK_MASK	(0x0000FFFF & ~_TIF_SECCOMP)
 
 /* flags to check in __switch_to() */
-#define _TIF_WORK_CTXSW (_TIF_DEBUG|_TIF_IO_BITMAP)
+#define _TIF_WORK_CTXSW_NEXT (_TIF_IO_BITMAP | _TIF_NOTSC | _TIF_DEBUG)
+#define _TIF_WORK_CTXSW_PREV (_TIF_IO_BITMAP | _TIF_NOTSC)
 
 /*
  * Thread-synchronous status.
diff -r 93feac10afde -r fa49c58866fe include/linux/prctl.h
--- a/include/linux/prctl.h	Tue Jul 25 11:05:21 2006 -0200
+++ b/include/linux/prctl.h	Tue Jul 25 23:33:52 2006 +0200
@@ -59,4 +59,8 @@
 # define PR_ENDIAN_LITTLE	1	/* True little endian mode */
 # define PR_ENDIAN_PPC_LITTLE	2	/* "PowerPC" pseudo little endian */
 
+/* Get/set process seccomp mode */
+#define PR_GET_SECCOMP	21
+#define PR_SET_SECCOMP	22
+
 #endif /* _LINUX_PRCTL_H */
diff -r 93feac10afde -r fa49c58866fe include/linux/seccomp.h
--- a/include/linux/seccomp.h	Tue Jul 25 11:05:21 2006 -0200
+++ b/include/linux/seccomp.h	Tue Jul 25 23:33:52 2006 +0200
@@ -3,8 +3,6 @@
 
 
 #ifdef CONFIG_SECCOMP
-
-#define NR_SECCOMP_MODES 1
 
 #include <linux/thread_info.h>
 #include <asm/seccomp.h>
@@ -18,20 +16,23 @@ static inline void secure_computing(int 
 		__secure_computing(this_syscall);
 }
 
-static inline int has_secure_computing(struct thread_info *ti)
-{
-	return unlikely(test_ti_thread_flag(ti, TIF_SECCOMP));
-}
+extern long prctl_get_seccomp(void);
+extern long prctl_set_seccomp(unsigned long);
 
 #else /* CONFIG_SECCOMP */
 
 typedef struct { } seccomp_t;
 
 #define secure_computing(x) do { } while (0)
-/* static inline to preserve typechecking */
-static inline int has_secure_computing(struct thread_info *ti)
+
+static inline long prctl_get_seccomp(void)
 {
-	return 0;
+	return -EINVAL;
+}
+
+static inline long prctl_set_seccomp(unsigned long arg2)
+{
+	return -EINVAL;
 }
 
 #endif /* CONFIG_SECCOMP */
diff -r 93feac10afde -r fa49c58866fe kernel/seccomp.c
--- a/kernel/seccomp.c	Tue Jul 25 11:05:21 2006 -0200
+++ b/kernel/seccomp.c	Tue Jul 25 23:33:52 2006 +0200
@@ -1,7 +1,7 @@
 /*
  * linux/kernel/seccomp.c
  *
- * Copyright 2004-2005  Andrea Arcangeli <andrea@cpushare.com>
+ * Copyright 2004-2006  Andrea Arcangeli <andrea@cpushare.com>
  *
  * This defines a simple but solid secure-computing mode.
  */
@@ -10,6 +10,7 @@
 #include <linux/sched.h>
 
 /* #define SECCOMP_DEBUG 1 */
+#define NR_SECCOMP_MODES 1
 
 /*
  * Secure computing mode 1 allows only read/write/exit/sigreturn.
@@ -54,3 +55,31 @@ void __secure_computing(int this_syscall
 #endif
 	do_exit(SIGKILL);
 }
+
+long prctl_get_seccomp(void)
+{
+	return current->seccomp.mode;
+}
+
+long prctl_set_seccomp(unsigned long seccomp_mode)
+{
+	long ret;
+
+	/* can set it only once to be even more secure */
+	ret = -EPERM;
+	if (unlikely(current->seccomp.mode))
+		goto out;
+
+	ret = -EINVAL;
+	if (seccomp_mode && seccomp_mode <= NR_SECCOMP_MODES) {
+		current->seccomp.mode = seccomp_mode;
+		set_thread_flag(TIF_SECCOMP);
+#ifdef TIF_NOTSC
+		disable_TSC();
+#endif
+		ret = 0;
+	}
+
+ out:
+	return ret;
+}
diff -r 93feac10afde -r fa49c58866fe kernel/sys.c
--- a/kernel/sys.c	Tue Jul 25 11:05:21 2006 -0200
+++ b/kernel/sys.c	Tue Jul 25 23:33:52 2006 +0200
@@ -28,6 +28,7 @@
 #include <linux/tty.h>
 #include <linux/signal.h>
 #include <linux/cn_proc.h>
+#include <linux/seccomp.h>
 
 #include <linux/compat.h>
 #include <linux/syscalls.h>
@@ -2056,6 +2057,13 @@ asmlinkage long sys_prctl(int option, un
 			error = SET_ENDIAN(current, arg2);
 			break;
 
+		case PR_GET_SECCOMP:
+			error = prctl_get_seccomp();
+			break;
+		case PR_SET_SECCOMP:
+			error = prctl_set_seccomp(arg2);
+			break;
+
 		default:
 			error = -EINVAL;
 			break;

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[Ingo Molnar]

* andrea@cpushare.com <andrea@cpushare.com> wrote:

> Here a repost of the last seccomp patch against current mainline 
> including the preempt fix. This changes the seccomp API from 
> /proc/<pid>/seccomp to a prctl (this will produce a smaller kernel) 
> and it adds a TIF_NOTSC that seccomp sets. Only the current task can 
> call disable_TSC (obviously because it hasn't a task_t param). This 
> includes Chuck's patch to give zero runtime cost to the notsc feature.

please send a patch-queue that is properly split-up: the bugfix, the API 
change and the TIF_NOTSC improvement.

	Ingo

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[andrea]

On Wed, Jul 26, 2006 at 10:07:39AM +0200, Ingo Molnar wrote:
> 
> * andrea@cpushare.com <andrea@cpushare.com> wrote:
> 
> > Here a repost of the last seccomp patch against current mainline 
> > including the preempt fix. This changes the seccomp API from 
> > /proc/<pid>/seccomp to a prctl (this will produce a smaller kernel) 
> > and it adds a TIF_NOTSC that seccomp sets. Only the current task can 
> > call disable_TSC (obviously because it hasn't a task_t param). This 
> > includes Chuck's patch to give zero runtime cost to the notsc feature.
> 
> please send a patch-queue that is properly split-up: the bugfix, the API 
> change and the TIF_NOTSC improvement.

Which bugfix do you mean? If you mean the preempt fix for the NOTSC
improvement it makes no sense to split it up from the NOTSC
part. There are no other bugfixes (the reduction of the notsc window
isn't strictly a bugfix, since the feature already helped).

I can split the API change from the NOTSC feature, I'll wait some more
days in the hope this one goes in. If it doesn't go in I'll follow
your suggestion and I'll try again later with the split up in the hope
to increase my chances.

>From my point of view it's not urgent to merge it, it's just the
anti-seccomp advocates that should want this patch being merged
urgently.

Re: [patch] let CONFIG_SECCOMP default to n

[andrea]

On Tue, Jul 11, 2006 at 09:36:25AM +0200, Ingo Molnar wrote:
> 
> * andrea@cpushare.com <andrea@cpushare.com> wrote:
> 
> > On Fri, Jun 30, 2006 at 11:47:53AM +0200, Ingo Molnar wrote:
> > > and both are pledged and available to GPL users. [..]
> > 
> > If the GPL offered any protection to my system software I would 
> > consider it too, but the GPL can't protect software that runs behind 
> > the corporate firewall. [...]
> 
> so you admit and confirm that you explicitly and intentionally do not 
> pledge your patent to GPL users. [..]

How many times do I need to say it. The pending patent has nothing to do
with the kernel, and it is _not_ pledged under the GPL.

The software world is moving from proprietary software that runs on top
of end user computers, to proprietary software that runs behind the
firewall. And the GPL is getting old and not capable to cope with the
second kind of usage of the software. This is being discussed in gplv3
context.

This isn't too bad, because as long as the software you run is open
source, you're guaranteed to be in control of what runs on your
computer, in control of your privacy, and in control of your security.
So it's a better model, you pratically run the proprietary software on
the server through the firefox open source client stack, but but the
general forced-contribution-mode that the GPL allows, tends to weaken
signficantly when software is only meant to run behind the firewall of
large corporations that don't depend on external companies for their own
software support.

I've no magic solution for this, I'm not a lawyer so I've no idea if
anything more than the GPL would be enforceable under US law in the
first place (until recently it wasn't even 100% sure that the GPL was
enforceable), so I certainly can't help on this side.

> [..] That is troubling (and unethical) in my 

You should talk about ethics to the people that written the law,
certainly not with me. I've to comply with the law and with the rules of
economy. It's not my fault the fact that if I don't file patents I risk
troubles ahead. I'm against patents too and it wasn't fun to pay for the
legal costs of the filing. I seem to recall that transmeta also filed a
software patent over the CMS but it seems people had not many problems
to work for them (you once sent emails @transmeta.com too). Transmeta
perhaps was the smallest player, there are many more bigger examples
that I won't be making.

Furthermore I obtained no patents yet, it could be the way I written the
text is wrong or whatever, I'm not the most expert in terms of patents,
I did my best just to be sure I couldn't regret having done a fatal
and obvious mistake later and it seems everything is going fine, but
doing my best is definitely no guarantee of success. So it could be all
this will be void if I've bad luck and they reject my application.

> opinion and strengthens my argument that this feature should be _at a 
> minimum_ be made default-off, just like hundreds of other kernel 
> features are.

See the below email of yours.

Also note, seccomp is the basic mode, at some point trusted
computing will be supported by xen so then I'll have a compelling reason
to switch the client side from seccomp to xen. But even then I'll be
always supporting seccomp for a long time because it's the most solid
and simplest mode of all.

> I'm also wondering what the upstream decision would have been, had you 
> disclosed this patent licensing intention of yours. (to use the GPL-ed 

Talking about patents when submitting seccomp, would be like talking
about mp3 patents when submitting alsa code or talking about google
server side patents when submitting a new tcp/ip feature that could
allow google render html faster. This whole discussion is officially
offtopic and it's a pure waste of bandwidth(tm), believe it or not.

All grid computing providers out there are welcome to start using
seccomp today to make their clients more secure against possible
software bugs in the remote computed bytecode. I welcome boinc to start
using seccomp too, I welcome worldcommunitygrid to use seccomp on the
linux client, I welcome all OS vendors to add seccomp to their OS, I
even tried to contact apple since it should be easy to port seccomp to
their kernels.

As far as cpushare is concerned, I never had anything to hide. The legal
part of the website is there since day zero I think.

> Linux kernel as a vehicle for your 'invention', while not fully living 
> up to the basic quid-pro-quo.).

If you prefer I can move all patent pending code on top of windows,
though I believe the "powered by" ads on my site were nice ads for free
software and open source (and obviously I'm very proud to be using them
like I'm very proud to be able to contribute to free software as well).
If you check the user agreement I even stated that any income generated
by cpucoins transactions started by cpushare will be donated to the
development of open source software.

> So i'd like to request the patch below to be included in v2.6.18.

Here the email that now you're forcing me to remind you:

http://www.cpushare.com/hypermail/cpushare-discuss/06/01/0080.html

	"ok, i agree with you here - having it on by default does make
	sense from an API uniformity POV."

I didn't feel the need of mentioning this opinion of yours before as
backing for my arguments pro Y, because I thought you were agreeing with
my previous post and that my arguments alone were enough, but since you
changed your mind again...

Note that I don't think Y or N makes any difference at the end for my
project. But fedora could set it to N under your advice and that would
do more damage to my project than whatever default setting we have
in-kernel. So if you want to hurt my project, you should ask Dave to
turn off seccomp instead of asking Linus to turn off it in the kernel
source.

Even more significant is that fact that it turns out 90% of the
interested userbase seems windows based, so for 90% of users it won't
matter if seccomp is even in the kernel.

Even if you seem to believe I don't care about the kernel when I talk
about seccomp, I really think Y is the right setting for the kernel, and
I'm not speaking for my own personal usages of seccomp, for the reason
why you also agreed with it in the above email a few months ago.

But like I said in previous emails, if these discussion about Y or N
have to keep going, then feel free to apply Ingo patch to make him
happy. I'm happy either ways even but I think Y is the appropriate
setting like with EPOLL and friends, now that all overhead triggered by
the purely paranoid additional feature has been removed by default.

config EPOLL
        bool "Enable eventpoll support" if EMBEDDED
        default y

Re: [patch] let CONFIG_SECCOMP default to n

[Arjan van de Ven]

> Note that I don't think Y or N makes any difference at the end for my
> project. But fedora could set it to N under your advice and that would
> do more damage to my project than whatever default setting we have

as far as I can see Fedora has SECCOMP off for a long time already

> Even if you seem to believe I don't care about the kernel when I talk
> about seccomp, I really think Y is the right setting for the kernel, and
> I'm not speaking for my own personal usages of seccomp, for the reason
> why you also agreed with it in the above email a few months ago.

if there is overhead, and there is no general use for it (which there
isn't really) then it should be off imo.

Re: [patch] let CONFIG_SECCOMP default to n

[andrea]

On Tue, Jul 11, 2006 at 04:32:53PM +0200, Arjan van de Ven wrote:
> as far as I can see Fedora has SECCOMP off for a long time already

Well, I didn't know about it... Long time can't be more than a few
months because I was sure in older releases it was enabled because I had
people running seccomp code on fedora.

I never expect it was easy thing to startup the CPUShare project, but
one thing that I didn't expect however was this kind of behaviour from
the leading linux vendor, I didn't get a single email of questions and I
wasn't informed about this, despite they know me perfectly. This
effectively reminds me about the high profile news articles I keep
reading recently that on the sidelines mentions about some RH behaviour
in the industry.

> if there is overhead, and there is no general use for it (which there
> isn't really) then it should be off imo.

I hope the reason was the lack of my last patch. But even in such case
RH could have turned off the tsc thing immediately themself (they know
how to patch the kernel no?) or they could have asked me a single
question about it before turning it off, no?

I hope RH will reconsider with my last patch applied and at the light of
this email as well:

	http://www.cpushare.com/hypermail/cpushare-discuss/06/01/0080.html

If they don't reconsider I'll be forced to recommend the Fedora CPUShare
users to switch distro if they don't want having to recompile the kernel
by themself.

I guess now I understand why this new change of mind of Ingo: if he
would succeed to push the N in the main kernel, then nobody could
complain to fedora for setting it to N, while they're in a less obvious
position at the moment where the kernel says "default to y" and they set
it to N to be happy.

As for no general use, this is the people that certainly used seccomp so
far:

cpushare=> select count(*) from accounts where cpucoins != 0;
 count 
-------
   122
(1 row)

cpushare=> 

remove 1 that is myself, that leaves 121 persons using seccomp so far
in CPUShare context. One first user already started buying CPU resource
a few days ago, and he's currently computing his own seccomp bytecode
remotely as we speak. So unless they're all wasting their time by
helping me testing the stuff, I'm not the only one that find at least
one useful usage for seccomp (but I think there are many more if only
people would care to use it). Certainly the FUD about the Y and N
availability doesn't help in convincing people to use seccomp to
strengthen decompression security etc...

Re: [patch] let CONFIG_SECCOMP default to n

[Alan Cox]

Ar Maw, 2006-07-11 am 17:31 +0200, ysgrifennodd andrea@cpushare.com:
> If they don't reconsider I'll be forced to recommend the Fedora CPUShare
> users to switch distro if they don't want having to recompile the kernel
> by themself.

I'm sure they'll both be deeply upset.

I really don't care about cpushare and patents for some users of the
code in question. On the other hand turning on performance harming code
for a tiny number of users is dumb. If it were a loadable module it
would be different.


Alan

Re: [patch] let CONFIG_SECCOMP default to n

[Andi Kleen]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> 
> I really don't care about cpushare and patents for some users of the
> code in question. On the other hand turning on performance harming code
> for a tiny number of users is dumb. If it were a loadable module it
> would be different.

Actually there are some promising applications of seccomp outside
cpushare.

e.g. Andrea at some point proposed to run codecs which often
have security issues in a simple cpusec jail.  That's ok for 
them because they normally don't need to do any system calls.

I liked the idea. While this can be done with LSM (e.g. apparmor) too 
seccomp is definitely much easier and simpler and more "obviously safe"
than anything LSM based.

If the TSC disabling code is taken out the runtime overhead
of seccomp is also very small because it's only tested in slow
paths.

-Andi

Re: [patch] let CONFIG_SECCOMP default to n

[Ingo Molnar]

* Andi Kleen <ak@suse.de> wrote:

> If the TSC disabling code is taken out the runtime overhead of seccomp 
> is also very small because it's only tested in slow paths.

correct. But when i suggested to do precisely that i got a rant from 
Andrea of how super duper important it was to disable the TSC for 
seccomp ... (which argument is almost total hogwash)

so i'm going with the simpler path of making seccomp default-off. (which 
solves the problem as far as i'm concerned - i.e. no default overhead in 
the scheduler.)

but Andrea's creative arguments wrt. his decision to not pledge the 
seccomp related patent to GPL users makes me worry about whether this 
technology is untainted. We _dont_ want to make Linux reliant on 
possibly hostile patents - especially not for core default-enabled 
functionality. Basically Andrea wants us to help his project but he is 
in essence rejecting to do the same for us (for one of the most obvious 
applications of trusted bytecode: over-the-internet clustering via 
seccomp) - i find that approach fundamentally unfair. And he has not 
given a satisfactory answer either - IMO his 'GPL is not good because of 
the corporate firewall licensing backhole' argument is ridiculous [if he 
doesnt like the GPL he should write his own OS i guess, and not try to 
use a GPL-ed kernel as a vehicle for his technology]. (And he has 
injected his code into code that i authored too, which makes it doubly 
offensive to me.)

the fundamental problem is what i sense to be arrogant behavior of 
Andrea all across. I reported a performance problem months ago with two 
simple patches (the first one fixing seccomp, the second one disabling 
it by default) to get rid of the problem, and what i got was insults 
from Andrea and hours spent on writing pointless emails. Andrea is 
forcing me to invest time into this stupidity and that just increases my 
sense of being abused. I also start being of the opinion that no matter 
how good of a coder Andrea is, i dont want to deal with his code _at 
all_ if such _basic_ issues like performance regressions are so hard to 
communicate. At a minimum Andrea should apologize for all that abuse 
that i got just because i happened to cross his tracks with his 
holy-grail patent-pending technology.

another problem is the double standard Andrea's code is enjoying. 
Despite good resons to apply the patch, it has not been applied yet, 
with no explanation. Again, i request the patch below to be applied to 
the upstream kernel. If Andrea fixes the performance problem and fixes 
the patent taining issue we can turn the feature back on. Is Andrea's 
code above the rules of maintainance?

really, how much more stupid can the situation get before we get a 
resolution?

And i just wasted another 15 minutes on this ...

	Ingo

---------------->
From: Ingo Molnar <mingo@elte.hu>
Subject: let CONFIG_SECCOMP default to n

I was profiling the scheduler on x86 and noticed some overhead related 
to SECCOMP, and indeed, SECCOMP runs disable_tsc() at _every_ 
context-switch:

        if (unlikely(prev->io_bitmap_ptr || next->io_bitmap_ptr))
                handle_io_bitmap(next, tss);

        disable_tsc(prev_p, next_p);

        return prev_p;

these are a couple of instructions in the hottest scheduler codepath!

x86_64 already removed disable_tsc() from switch_to(), but i think the 
right solution is to turn SECCOMP off by default.

besides the runtime overhead, there are a couple of other reasons as 
well why this should be done:

 - CONFIG_SECCOMP=y adds 836 bytes of bloat to the kernel:

       text    data     bss     dec     hex filename
    4185360  867112  391012 5443484  530f9c vmlinux-noseccomp
    4185992  867316  391012 5444320  5312e0 vmlinux-seccomp

 - virtually nobody seems to be using it (but cpushare.com, which seems
   pretty inactive)

 - users/distributions can still turn it on if they want it

 - http://www.cpushare.com/legal seems to suggest that it is pursuing a
   software patent to utilize the seccomp concept in a distributed 
   environment, and seems to give a promise that 'end users' will not be
   affected by that patent. How about non-end-users [i.e. server-side]?
   Has the Linux kernel become a vehicle for a propriety server-side
   feature, with every Linux user paying the price of it?

so the patch below just does the minimal common-sense change: turn it 
off by default.

Adrian Bunk:
I've removed the superfluous "default n"'s the original patch introduced.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Adrian Bunk <bunk@stusta.de>

----

This patch was already sent on:
- 19 Apr 2006
- 11 Apr 2006
- 10 Mar 2006
- 29 Jan 2006
- 21 Jan 2006

This patch was sent by Ingo Molnar on:
- 9 Jan 2006

Index: linux/arch/i386/Kconfig
===================================================================
--- linux.orig/arch/i386/Kconfig
+++ linux/arch/i386/Kconfig
@@ -637,7 +637,6 @@ config REGPARM
 config SECCOMP
 	bool "Enable seccomp to safely compute untrusted bytecode"
 	depends on PROC_FS
-	default y
 	help
 	  This kernel feature is useful for number crunching applications
 	  that may need to compute untrusted bytecode during their
Index: linux/arch/mips/Kconfig
===================================================================
--- linux.orig/arch/mips/Kconfig
+++ linux/arch/mips/Kconfig
@@ -1787,7 +1787,6 @@ config BINFMT_ELF32
 config SECCOMP
 	bool "Enable seccomp to safely compute untrusted bytecode"
 	depends on PROC_FS && BROKEN
-	default y
 	help
 	  This kernel feature is useful for number crunching applications
 	  that may need to compute untrusted bytecode during their
Index: linux/arch/powerpc/Kconfig
===================================================================
--- linux.orig/arch/powerpc/Kconfig
+++ linux/arch/powerpc/Kconfig
@@ -666,7 +666,6 @@ endif
 config SECCOMP
 	bool "Enable seccomp to safely compute untrusted bytecode"
 	depends on PROC_FS
-	default y
 	help
 	  This kernel feature is useful for number crunching applications
 	  that may need to compute untrusted bytecode during their
Index: linux/arch/ppc/Kconfig
===================================================================
--- linux.orig/arch/ppc/Kconfig
+++ linux/arch/ppc/Kconfig
@@ -1127,7 +1127,6 @@ endif
 config SECCOMP
 	bool "Enable seccomp to safely compute untrusted bytecode"
 	depends on PROC_FS
-	default y
 	help
 	  This kernel feature is useful for number crunching applications
 	  that may need to compute untrusted bytecode during their
Index: linux/arch/sparc64/Kconfig
===================================================================
--- linux.orig/arch/sparc64/Kconfig
+++ linux/arch/sparc64/Kconfig
@@ -64,7 +64,6 @@ endchoice
 config SECCOMP
 	bool "Enable seccomp to safely compute untrusted bytecode"
 	depends on PROC_FS
-	default y
 	help
 	  This kernel feature is useful for number crunching applications
 	  that may need to compute untrusted bytecode during their
Index: linux/arch/x86_64/Kconfig
===================================================================
--- linux.orig/arch/x86_64/Kconfig
+++ linux/arch/x86_64/Kconfig
@@ -466,7 +466,6 @@ config PHYSICAL_START
 config SECCOMP
 	bool "Enable seccomp to safely compute untrusted bytecode"
 	depends on PROC_FS
-	default y
 	help
 	  This kernel feature is useful for number crunching applications
 	  that may need to compute untrusted bytecode during their

Re: [patch] let CONFIG_SECCOMP default to n

[Andrew Morton]

On Wed, 12 Jul 2006 23:07:32 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> Despite good resons to apply the patch, it has not been applied yet, 
> with no explanation.

I queued the below.  Andrea claims that it'll reduce seccomp overhead to
literally zero.

But looking at it, I think it's a bit confused.  The patch needs
s/DISABLE_TSC/ENABLE_TSC/ to make it right.





From: Andrea Arcangeli <andrea@cpushare.com>

Make the TSC disable purely paranoid feature optional, so by default seccomp
returns absolutely zerocost.

Signed-off-by: Andrea Arcangeli <andrea@cpushare.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 arch/i386/Kconfig            |   12 ++++++++++++
 arch/i386/kernel/process.c   |    2 ++
 arch/x86_64/Kconfig          |   12 ++++++++++++
 arch/x86_64/kernel/process.c |   31 +++++++++++++++++++++++++++++++
 4 files changed, 57 insertions(+)

diff -puN arch/i386/Kconfig~add-seccomp_disable_tsc-config-option arch/i386/Kconfig
--- a/arch/i386/Kconfig~add-seccomp_disable_tsc-config-option
+++ a/arch/i386/Kconfig
@@ -737,6 +737,18 @@ config SECCOMP
 
 	  If unsure, say Y. Only embedded should say N here.
 
+config SECCOMP_DISABLE_TSC
+	bool "Disable the TSC for seccomp tasks"
+	depends on SECCOMP
+	default n
+	help
+	  This feature mathematically prevents covert channels
+	  for tasks running under SECCOMP. This can generate
+	  a minuscule overhead in the scheduler.
+
+	  If you care most about performance say N. Say Y only if you're
+	  paranoid about covert channels.
+
 config VGA_NOPROBE
        bool "Don't probe VGA at boot" if EMBEDDED
        default n
diff -puN arch/i386/kernel/process.c~add-seccomp_disable_tsc-config-option arch/i386/kernel/process.c
--- a/arch/i386/kernel/process.c~add-seccomp_disable_tsc-config-option
+++ a/arch/i386/kernel/process.c
@@ -572,6 +572,7 @@ handle_io_bitmap(struct thread_struct *n
 static inline void disable_tsc(struct task_struct *prev_p,
 			       struct task_struct *next_p)
 {
+#ifdef CONFIG_SECCOMP_DISABLE_TSC
 	struct thread_info *prev, *next;
 
 	/*
@@ -590,6 +591,7 @@ static inline void disable_tsc(struct ta
 			   has_secure_computing(next))
 			write_cr4(read_cr4() | X86_CR4_TSD);
 	}
+#endif
 }
 
 /*
diff -puN arch/x86_64/Kconfig~add-seccomp_disable_tsc-config-option arch/x86_64/Kconfig
--- a/arch/x86_64/Kconfig~add-seccomp_disable_tsc-config-option
+++ a/arch/x86_64/Kconfig
@@ -526,6 +526,18 @@ config SECCOMP
 
 	  If unsure, say Y. Only embedded should say N here.
 
+config SECCOMP_DISABLE_TSC
+	bool "Disable the TSC for seccomp tasks"
+	depends on SECCOMP
+	default n
+	help
+	  This feature mathematically prevents covert channels
+	  for tasks running under SECCOMP. This can generate
+	  a minuscule overhead in the scheduler.
+
+	  If you care most about performance say N. Say Y only if you're
+	  paranoid about covert channels.
+
 source kernel/Kconfig.hz
 
 config REORDER
diff -puN arch/x86_64/kernel/process.c~add-seccomp_disable_tsc-config-option arch/x86_64/kernel/process.c
--- a/arch/x86_64/kernel/process.c~add-seccomp_disable_tsc-config-option
+++ a/arch/x86_64/kernel/process.c
@@ -494,6 +494,35 @@ out:
 }
 
 /*
+ * This function selects if the context switch from prev to next
+ * has to tweak the TSC disable bit in the cr4.
+ */
+static inline void disable_tsc(struct task_struct *prev_p,
+			       struct task_struct *next_p)
+{
+#ifdef CONFIG_SECCOMP_DISABLE_TSC
+	struct thread_info *prev, *next;
+
+	/*
+	 * gcc should eliminate the ->thread_info dereference if
+	 * has_secure_computing returns 0 at compile time (SECCOMP=n).
+	 */
+	prev = prev_p->thread_info;
+	next = next_p->thread_info;
+
+	if (has_secure_computing(prev) || has_secure_computing(next)) {
+		/* slow path here */
+		if (has_secure_computing(prev) &&
+		    !has_secure_computing(next)) {
+			write_cr4(read_cr4() & ~X86_CR4_TSD);
+		} else if (!has_secure_computing(prev) &&
+			   has_secure_computing(next))
+			write_cr4((read_cr4() | X86_CR4_TSD) & ~X86_CR4_PCE);
+	}
+#endif
+}
+
+/*
  * This special macro can be used to load a debugging register
  */
 #define loaddebug(thread,r) set_debugreg(thread->debugreg ## r, r)
@@ -622,6 +651,8 @@ __switch_to(struct task_struct *prev_p, 
 		}
 	}
 
+	disable_tsc(prev_p, next_p);
+
 	/* If the task has used fpu the last 5 timeslices, just do a full
 	 * restore of the math state immediately to avoid the trap; the
 	 * chances of needing FPU soon are obviously high now
_

Re: [patch] let CONFIG_SECCOMP default to n

[James Bruce]

Andrew Morton wrote:
> On Wed, 12 Jul 2006 23:07:32 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
> 
>> Despite good resons to apply the patch, it has not been applied yet, 
>> with no explanation.
> 
> I queued the below.  Andrea claims that it'll reduce seccomp overhead to
> literally zero.
> 
> But looking at it, I think it's a bit confused.  The patch needs
> s/DISABLE_TSC/ENABLE_TSC/ to make it right.
<-- snip -->

Andrea,
what happened to Andrew James Wade's rewording [1] of your config help? 
   It seemed to disappear from what was submitted to akpm.

To "mathematically prevent covert channels" is far too strong a claim to 
make, since you only handle the case of TSC-related timing attacks. 
AJW's wording is much better, so please don't drop it.

Of course, if the new wording will be included in some forthcoming patch 
that also makes Linus happy [2], then never mind.

  - Jim Bruce

[1] http://lkml.org/lkml/2006/7/10/440
[2] http://lkml.org/lkml/2006/7/12/328

Re: [patch] let CONFIG_SECCOMP default to n

[andrea]

On Thu, Jul 13, 2006 at 03:44:38AM -0400, James Bruce wrote:
> Andrea,
> what happened to Andrew James Wade's rewording [1] of your config help? 
>   It seemed to disappear from what was submitted to akpm.

Andrew picked the patch I made originally, before Andrew James Wade
patched it.

Both patches are obsoleted by the new logic in the context switch that
uses the bitflags to enter the slow path, see Chuck's patch. That will
prevent the need of a config option because it's zero cost like the
core of seccomp.

As long as seccomp won't be nuked from the kernel, Chuck's patch seems
the way to go.

But the point is that I've no idea anymore what will happen to
seccomp so perhaps all patches will be useless.

Re: [patch] let CONFIG_SECCOMP default to n

[Andrew Morton]

On Thu, 13 Jul 2006 10:34:41 +0200
andrea@cpushare.com wrote:

> Both patches are obsoleted by the new logic in the context switch that
> uses the bitflags to enter the slow path, see Chuck's patch.

What darn patch?

<looks>

hm, p73wtain80h.fsf@verdi.suse.de, who appears to be Andi has (again)
removed me from cc.  Possibly an act of mercy ;)

> As long as seccomp won't be nuked from the kernel, Chuck's patch seems
> the way to go.

I see "[compile tested only; requires just-sent fix to i386 system.h]", so
an appropriate next step would be for you to review, test, sign-off and
forward it, please.

> But the point is that I've no idea anymore what will happen to
> seccomp so perhaps all patches will be useless.

Shrug.  If we can optimise the current code, fine.  If there's a default-on
config option that makes no-TSC seccomp have zero overhead, better.  If that
makes us go back to doing useful stuff, perfect.

[PATCH] TIF_NOTSC and SECCOMP prctl

[andrea]

On Thu, Jul 13, 2006 at 02:18:18AM -0700, Andrew Morton wrote:
> removed me from cc.  Possibly an act of mercy ;)

;)

> I see "[compile tested only; requires just-sent fix to i386 system.h]", so
> an appropriate next step would be for you to review, test, sign-off and
> forward it, please.

I took the liberty to add Chuck's signoff as well since I started
hacking on top of his patch, if this is not ok Chuck please let us
know.

The below patch seems to work, I ported all my client code on top of
prctl already. (it's a bit more painful to autodetect a kernel with
CONFIG_SECCOMP turned off but I already adapted to it)

The only thing left worth discussing is why if I set TIF_NOTSC to 10
instead of 19 the kernel was crashing hard... After I checked and
rechecked everything else I deduced it had to be that number and after
changing it to 19 everything works fine... I also verified the first
rdtsc kills the task with a sigsegv. It would be nice to make sure
it's not a bug in the below patch that 10 didn't work but just some
hidden kernel "feature" ;).

The reduction of 36 lines should be a welcome thing. I also left a
CONFIG_SECCOMP in the slow path around the TIF_NOTSC stuff, so the ones
setting CONFIG_SECCOMP=n won't notice any bytecode size
difference. (those two CONFIG_SECCOMP should be removed if somebody
adds a standalone prctl that only calls disable_TSC()).

Compared to Chuck's patch I also moved the io_bitmap in a path that
only executes if either prev or next have the TIF_IO_BITMAP set, which
seems more optimal.

Reviews are welcome (then I will move into x86-64, all other archs
supporting seccomp should require no changes despite the API
change). Thanks.

 arch/i386/kernel/process.c     |  124 +++++++++++++++++++++--------------------
 fs/proc/base.c                 |   91 ------------------------------
 include/asm-i386/processor.h   |    4 +
 include/asm-i386/thread_info.h |    5 +
 include/linux/prctl.h          |    4 +
 include/linux/seccomp.h        |   19 +++---
 kernel/seccomp.c               |   31 +++++++++-
 kernel/sys.c                   |    8 ++
 8 files changed, 125 insertions(+), 161 deletions(-)

Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>
Signed-off-by: Andrea Arcangeli <andrea@cpushare.com>

# HG changeset patch
# User andrea@cpushare.com
# Date 1152856077 -7200
# Node ID 9be99cbb325935c2a7af96ac39411fdde58d4eef
# Parent  bcfd682ea605a2ab00469eaa875988de6b910814
Removes the overhead of disabling the TSC under SECCCOMP
with a new TIF_NOTSC bitflag (idea and part of the code from Chuck Ebbert).
disable_TSC can be called by other kernel code without interfering with SECCOMP
in any way. A prctl could be added just to disable the TSC if anybody needs it.
Only the "current" task can call disable_TSC.

To reduce the bytes of .text to the minimum, the seccomp API is moved from
/proc to prctl. /proc wasn't necessary anymore because only the "current" task
can safely turn on the NOTSC bit without SMP race conditions.

diff -r bcfd682ea605 -r 9be99cbb3259 arch/i386/kernel/process.c
--- a/arch/i386/kernel/process.c	Thu Jul 13 03:03:35 2006 +0700
+++ b/arch/i386/kernel/process.c	Fri Jul 14 07:47:57 2006 +0200
@@ -535,8 +535,29 @@ int dump_task_regs(struct task_struct *t
 	return 1;
 }
 
-static noinline void __switch_to_xtra(struct task_struct *next_p,
-				    struct tss_struct *tss)
+#ifdef CONFIG_SECCOMP
+void hard_disable_TSC(void)
+{
+	write_cr4(read_cr4() | X86_CR4_TSD);
+}
+void disable_TSC(void)
+{
+	if (!test_and_set_thread_flag(TIF_NOTSC))
+		/*
+		 * Must flip the CPU state synchronously with
+		 * TIF_NOTSC in the current running context.
+		 */
+		hard_disable_TSC();
+}
+void hard_enable_TSC(void)
+{
+	write_cr4(read_cr4() & ~X86_CR4_TSD);
+}
+#endif /* CONFIG_SECCOMP */
+
+static noinline void
+__switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
+		 struct tss_struct *tss)
 {
 	struct thread_struct *next;
 
@@ -552,60 +573,47 @@ static noinline void __switch_to_xtra(st
 		set_debugreg(next->debugreg[7], 7);
 	}
 
-	if (!test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
+#ifdef CONFIG_SECCOMP
+	if (test_tsk_thread_flag(prev_p, TIF_NOTSC) ^
+	    test_tsk_thread_flag(next_p, TIF_NOTSC)) {
+		/* prev and next are different */
+		if (test_tsk_thread_flag(next_p, TIF_NOTSC))
+			hard_disable_TSC();
+		else
+			hard_enable_TSC();
+	}
+#endif
+
+	if (test_tsk_thread_flag(prev_p, TIF_IO_BITMAP) ||
+	    test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
+		if (!test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
+			/*
+			 * Disable the bitmap via an invalid offset. We still cache
+			 * the previous bitmap owner and the IO bitmap contents:
+			 */
+			tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
+			return;
+		}
+
+		if (likely(next == tss->io_bitmap_owner)) {
+			/*
+			 * Previous owner of the bitmap (hence the bitmap content)
+			 * matches the next task, we dont have to do anything but
+			 * to set a valid offset in the TSS:
+			 */
+			tss->io_bitmap_base = IO_BITMAP_OFFSET;
+			return;
+		}
 		/*
-		 * Disable the bitmap via an invalid offset. We still cache
-		 * the previous bitmap owner and the IO bitmap contents:
+		 * Lazy TSS's I/O bitmap copy. We set an invalid offset here
+		 * and we let the task to get a GPF in case an I/O instruction
+		 * is performed.  The handler of the GPF will verify that the
+		 * faulting task has a valid I/O bitmap and, it true, does the
+		 * real copy and restart the instruction.  This will save us
+		 * redundant copies when the currently switched task does not
+		 * perform any I/O during its timeslice.
 		 */
-		tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
-		return;
-	}
-
-	if (likely(next == tss->io_bitmap_owner)) {
-		/*
-		 * Previous owner of the bitmap (hence the bitmap content)
-		 * matches the next task, we dont have to do anything but
-		 * to set a valid offset in the TSS:
-		 */
-		tss->io_bitmap_base = IO_BITMAP_OFFSET;
-		return;
-	}
-	/*
-	 * Lazy TSS's I/O bitmap copy. We set an invalid offset here
-	 * and we let the task to get a GPF in case an I/O instruction
-	 * is performed.  The handler of the GPF will verify that the
-	 * faulting task has a valid I/O bitmap and, it true, does the
-	 * real copy and restart the instruction.  This will save us
-	 * redundant copies when the currently switched task does not
-	 * perform any I/O during its timeslice.
-	 */
-	tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET_LAZY;
-}
-
-/*
- * This function selects if the context switch from prev to next
- * has to tweak the TSC disable bit in the cr4.
- */
-static inline void disable_tsc(struct task_struct *prev_p,
-			       struct task_struct *next_p)
-{
-	struct thread_info *prev, *next;
-
-	/*
-	 * gcc should eliminate the ->thread_info dereference if
-	 * has_secure_computing returns 0 at compile time (SECCOMP=n).
-	 */
-	prev = task_thread_info(prev_p);
-	next = task_thread_info(next_p);
-
-	if (has_secure_computing(prev) || has_secure_computing(next)) {
-		/* slow path here */
-		if (has_secure_computing(prev) &&
-		    !has_secure_computing(next)) {
-			write_cr4(read_cr4() & ~X86_CR4_TSD);
-		} else if (!has_secure_computing(prev) &&
-			   has_secure_computing(next))
-			write_cr4(read_cr4() | X86_CR4_TSD);
+		tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET_LAZY;
 	}
 }
 
@@ -690,11 +698,9 @@ struct task_struct fastcall * __switch_t
 	/*
 	 * Now maybe handle debug registers and/or IO bitmaps
 	 */
-	if (unlikely((task_thread_info(next_p)->flags & _TIF_WORK_CTXSW))
-	    || test_tsk_thread_flag(prev_p, TIF_IO_BITMAP))
-		__switch_to_xtra(next_p, tss);
-
-	disable_tsc(prev_p, next_p);
+	if (unlikely(task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV ||
+		     task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT))
+		__switch_to_xtra(prev_p, next_p, tss);
 
 	return prev_p;
 }
diff -r bcfd682ea605 -r 9be99cbb3259 fs/proc/base.c
--- a/fs/proc/base.c	Thu Jul 13 03:03:35 2006 +0700
+++ b/fs/proc/base.c	Fri Jul 14 07:47:57 2006 +0200
@@ -67,7 +67,6 @@
 #include <linux/mount.h>
 #include <linux/security.h>
 #include <linux/ptrace.h>
-#include <linux/seccomp.h>
 #include <linux/cpuset.h>
 #include <linux/audit.h>
 #include <linux/poll.h>
@@ -98,9 +97,6 @@ enum pid_directory_inos {
 	PROC_TGID_TASK,
 	PROC_TGID_STATUS,
 	PROC_TGID_MEM,
-#ifdef CONFIG_SECCOMP
-	PROC_TGID_SECCOMP,
-#endif
 	PROC_TGID_CWD,
 	PROC_TGID_ROOT,
 	PROC_TGID_EXE,
@@ -141,9 +137,6 @@ enum pid_directory_inos {
 	PROC_TID_INO,
 	PROC_TID_STATUS,
 	PROC_TID_MEM,
-#ifdef CONFIG_SECCOMP
-	PROC_TID_SECCOMP,
-#endif
 	PROC_TID_CWD,
 	PROC_TID_ROOT,
 	PROC_TID_EXE,
@@ -212,9 +205,6 @@ static struct pid_entry tgid_base_stuff[
 	E(PROC_TGID_NUMA_MAPS, "numa_maps", S_IFREG|S_IRUGO),
 #endif
 	E(PROC_TGID_MEM,       "mem",     S_IFREG|S_IRUSR|S_IWUSR),
-#ifdef CONFIG_SECCOMP
-	E(PROC_TGID_SECCOMP,   "seccomp", S_IFREG|S_IRUSR|S_IWUSR),
-#endif
 	E(PROC_TGID_CWD,       "cwd",     S_IFLNK|S_IRWXUGO),
 	E(PROC_TGID_ROOT,      "root",    S_IFLNK|S_IRWXUGO),
 	E(PROC_TGID_EXE,       "exe",     S_IFLNK|S_IRWXUGO),
@@ -255,9 +245,6 @@ static struct pid_entry tid_base_stuff[]
 	E(PROC_TID_NUMA_MAPS,  "numa_maps",    S_IFREG|S_IRUGO),
 #endif
 	E(PROC_TID_MEM,        "mem",     S_IFREG|S_IRUSR|S_IWUSR),
-#ifdef CONFIG_SECCOMP
-	E(PROC_TID_SECCOMP,    "seccomp", S_IFREG|S_IRUSR|S_IWUSR),
-#endif
 	E(PROC_TID_CWD,        "cwd",     S_IFLNK|S_IRWXUGO),
 	E(PROC_TID_ROOT,       "root",    S_IFLNK|S_IRWXUGO),
 	E(PROC_TID_EXE,        "exe",     S_IFLNK|S_IRWXUGO),
@@ -970,78 +957,6 @@ static struct file_operations proc_login
 	.write		= proc_loginuid_write,
 };
 #endif
-
-#ifdef CONFIG_SECCOMP
-static ssize_t seccomp_read(struct file *file, char __user *buf,
-			    size_t count, loff_t *ppos)
-{
-	struct task_struct *tsk = get_proc_task(file->f_dentry->d_inode);
-	char __buf[20];
-	loff_t __ppos = *ppos;
-	size_t len;
-
-	if (!tsk)
-		return -ESRCH;
-	/* no need to print the trailing zero, so use only len */
-	len = sprintf(__buf, "%u\n", tsk->seccomp.mode);
-	put_task_struct(tsk);
-	if (__ppos >= len)
-		return 0;
-	if (count > len - __ppos)
-		count = len - __ppos;
-	if (copy_to_user(buf, __buf + __ppos, count))
-		return -EFAULT;
-	*ppos = __ppos + count;
-	return count;
-}
-
-static ssize_t seccomp_write(struct file *file, const char __user *buf,
-			     size_t count, loff_t *ppos)
-{
-	struct task_struct *tsk = get_proc_task(file->f_dentry->d_inode);
-	char __buf[20], *end;
-	unsigned int seccomp_mode;
-	ssize_t result;
-
-	result = -ESRCH;
-	if (!tsk)
-		goto out_no_task;
-
-	/* can set it only once to be even more secure */
-	result = -EPERM;
-	if (unlikely(tsk->seccomp.mode))
-		goto out;
-
-	result = -EFAULT;
-	memset(__buf, 0, sizeof(__buf));
-	count = min(count, sizeof(__buf) - 1);
-	if (copy_from_user(__buf, buf, count))
-		goto out;
-
-	seccomp_mode = simple_strtoul(__buf, &end, 0);
-	if (*end == '\n')
-		end++;
-	result = -EINVAL;
-	if (seccomp_mode && seccomp_mode <= NR_SECCOMP_MODES) {
-		tsk->seccomp.mode = seccomp_mode;
-		set_tsk_thread_flag(tsk, TIF_SECCOMP);
-	} else
-		goto out;
-	result = -EIO;
-	if (unlikely(!(end - __buf)))
-		goto out;
-	result = end - __buf;
-out:
-	put_task_struct(tsk);
-out_no_task:
-	return result;
-}
-
-static struct file_operations proc_seccomp_operations = {
-	.read		= seccomp_read,
-	.write		= seccomp_write,
-};
-#endif /* CONFIG_SECCOMP */
 
 static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
@@ -1726,12 +1641,6 @@ static struct dentry *proc_pident_lookup
 		case PROC_TGID_MEM:
 			inode->i_fop = &proc_mem_operations;
 			break;
-#ifdef CONFIG_SECCOMP
-		case PROC_TID_SECCOMP:
-		case PROC_TGID_SECCOMP:
-			inode->i_fop = &proc_seccomp_operations;
-			break;
-#endif /* CONFIG_SECCOMP */
 		case PROC_TID_MOUNTS:
 		case PROC_TGID_MOUNTS:
 			inode->i_fop = &proc_mounts_operations;
diff -r bcfd682ea605 -r 9be99cbb3259 include/asm-i386/processor.h
--- a/include/asm-i386/processor.h	Thu Jul 13 03:03:35 2006 +0700
+++ b/include/asm-i386/processor.h	Fri Jul 14 07:47:57 2006 +0200
@@ -256,6 +256,10 @@ static inline void clear_in_cr4 (unsigne
 	cr4 &= ~mask;
 	write_cr4(cr4);
 }
+
+extern void hard_disable_TSC(void);
+extern void disable_TSC(void);
+extern void hard_enable_TSC(void);
 
 /*
  *      NSC/Cyrix CPU configuration register indexes
diff -r bcfd682ea605 -r 9be99cbb3259 include/asm-i386/thread_info.h
--- a/include/asm-i386/thread_info.h	Thu Jul 13 03:03:35 2006 +0700
+++ b/include/asm-i386/thread_info.h	Fri Jul 14 07:47:57 2006 +0200
@@ -142,6 +142,7 @@ static inline struct thread_info *curren
 #define TIF_MEMDIE		16
 #define TIF_DEBUG		17	/* uses debug registers */
 #define TIF_IO_BITMAP		18	/* uses I/O bitmap */
+#define TIF_NOTSC		19	/* TSC is not accessible in userland */
 
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
 #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
@@ -153,6 +154,7 @@ static inline struct thread_info *curren
 #define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1<<TIF_SECCOMP)
 #define _TIF_RESTORE_SIGMASK	(1<<TIF_RESTORE_SIGMASK)
+#define _TIF_NOTSC		(1<<TIF_NOTSC)
 #define _TIF_DEBUG		(1<<TIF_DEBUG)
 #define _TIF_IO_BITMAP		(1<<TIF_IO_BITMAP)
 
@@ -164,7 +166,8 @@ static inline struct thread_info *curren
 #define _TIF_ALLWORK_MASK	(0x0000FFFF & ~_TIF_SECCOMP)
 
 /* flags to check in __switch_to() */
-#define _TIF_WORK_CTXSW (_TIF_DEBUG|_TIF_IO_BITMAP)
+#define _TIF_WORK_CTXSW_NEXT (_TIF_IO_BITMAP | _TIF_NOTSC | _TIF_DEBUG)
+#define _TIF_WORK_CTXSW_PREV (_TIF_IO_BITMAP | _TIF_NOTSC)
 
 /*
  * Thread-synchronous status.
diff -r bcfd682ea605 -r 9be99cbb3259 include/linux/prctl.h
--- a/include/linux/prctl.h	Thu Jul 13 03:03:35 2006 +0700
+++ b/include/linux/prctl.h	Fri Jul 14 07:47:57 2006 +0200
@@ -59,4 +59,8 @@
 # define PR_ENDIAN_LITTLE	1	/* True little endian mode */
 # define PR_ENDIAN_PPC_LITTLE	2	/* "PowerPC" pseudo little endian */
 
+/* Get/set process seccomp mode */
+#define PR_GET_SECCOMP	21
+#define PR_SET_SECCOMP	22
+
 #endif /* _LINUX_PRCTL_H */
diff -r bcfd682ea605 -r 9be99cbb3259 include/linux/seccomp.h
--- a/include/linux/seccomp.h	Thu Jul 13 03:03:35 2006 +0700
+++ b/include/linux/seccomp.h	Fri Jul 14 07:47:57 2006 +0200
@@ -3,8 +3,6 @@
 
 
 #ifdef CONFIG_SECCOMP
-
-#define NR_SECCOMP_MODES 1
 
 #include <linux/thread_info.h>
 #include <asm/seccomp.h>
@@ -18,20 +16,23 @@ static inline void secure_computing(int 
 		__secure_computing(this_syscall);
 }
 
-static inline int has_secure_computing(struct thread_info *ti)
-{
-	return unlikely(test_ti_thread_flag(ti, TIF_SECCOMP));
-}
+extern long prctl_get_seccomp(void);
+extern long prctl_set_seccomp(unsigned long);
 
 #else /* CONFIG_SECCOMP */
 
 typedef struct { } seccomp_t;
 
 #define secure_computing(x) do { } while (0)
-/* static inline to preserve typechecking */
-static inline int has_secure_computing(struct thread_info *ti)
+
+static inline long prctl_get_seccomp(void)
 {
-	return 0;
+	return -EINVAL;
+}
+
+static inline long prctl_set_seccomp(unsigned long arg2)
+{
+	return -EINVAL;
 }
 
 #endif /* CONFIG_SECCOMP */
diff -r bcfd682ea605 -r 9be99cbb3259 kernel/seccomp.c
--- a/kernel/seccomp.c	Thu Jul 13 03:03:35 2006 +0700
+++ b/kernel/seccomp.c	Fri Jul 14 07:47:57 2006 +0200
@@ -1,7 +1,7 @@
 /*
  * linux/kernel/seccomp.c
  *
- * Copyright 2004-2005  Andrea Arcangeli <andrea@cpushare.com>
+ * Copyright 2004-2006  Andrea Arcangeli <andrea@cpushare.com>
  *
  * This defines a simple but solid secure-computing mode.
  */
@@ -10,6 +10,7 @@
 #include <linux/sched.h>
 
 /* #define SECCOMP_DEBUG 1 */
+#define NR_SECCOMP_MODES 1
 
 /*
  * Secure computing mode 1 allows only read/write/exit/sigreturn.
@@ -54,3 +55,31 @@ void __secure_computing(int this_syscall
 #endif
 	do_exit(SIGKILL);
 }
+
+long prctl_get_seccomp(void)
+{
+	return current->seccomp.mode;
+}
+
+long prctl_set_seccomp(unsigned long seccomp_mode)
+{
+	long ret;
+
+	/* can set it only once to be even more secure */
+	ret = -EPERM;
+	if (unlikely(current->seccomp.mode))
+		goto out;
+
+	ret = -EINVAL;
+	if (seccomp_mode && seccomp_mode <= NR_SECCOMP_MODES) {
+		current->seccomp.mode = seccomp_mode;
+		set_thread_flag(TIF_SECCOMP);
+#ifdef TIF_NOTSC
+		disable_TSC();
+#endif
+		ret = 0;
+	}
+
+ out:
+	return ret;
+}
diff -r bcfd682ea605 -r 9be99cbb3259 kernel/sys.c
--- a/kernel/sys.c	Thu Jul 13 03:03:35 2006 +0700
+++ b/kernel/sys.c	Fri Jul 14 07:47:57 2006 +0200
@@ -28,6 +28,7 @@
 #include <linux/tty.h>
 #include <linux/signal.h>
 #include <linux/cn_proc.h>
+#include <linux/seccomp.h>
 
 #include <linux/compat.h>
 #include <linux/syscalls.h>
@@ -2056,6 +2057,13 @@ asmlinkage long sys_prctl(int option, un
 			error = SET_ENDIAN(current, arg2);
 			break;
 
+		case PR_GET_SECCOMP:
+			error = prctl_get_seccomp();
+			break;
+		case PR_SET_SECCOMP:
+			error = prctl_set_seccomp(arg2);
+			break;
+
 		default:
 			error = -EINVAL;
 			break;

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[Andrew Morton]

On Fri, 14 Jul 2006 08:09:32 +0200
andrea@cpushare.com wrote:

> The only thing left worth discussing is why if I set TIF_NOTSC to 10
> instead of 19 the kernel was crashing hard... After I checked and
> rechecked everything else I deduced it had to be that number and after
> changing it to 19 everything works fine... I also verified the first
> rdtsc kills the task with a sigsegv. It would be nice to make sure
> it's not a bug in the below patch that 10 didn't work but just some
> hidden kernel "feature" ;).

Using a bit <= 15 will cause kernel to take the work_notifysig path
"pending work-to-be-done flags are in LSW".  I'm not sure what happens if
there's such a flag set but nothing is set up to handle it.  I guess it
stays set and processes never get out of the kernel again.

Perhaps TIF_SECCOMP should be >= 16 too - the special-case in
_TIF_ALLWORK_MASK looks odd.

Re: [PATCH] TIF_NOTSC and SECCOMP prctl

[andrea]

On Thu, Jul 13, 2006 at 11:27:27PM -0700, Andrew Morton wrote:
> Using a bit <= 15 will cause kernel to take the work_notifysig path
> "pending work-to-be-done flags are in LSW".  I'm not sure what happens if
> there's such a flag set but nothing is set up to handle it.  I guess it
> stays set and processes never get out of the kernel again.

Ah ok, thanks.

> Perhaps TIF_SECCOMP should be >= 16 too - the special-case in
> _TIF_ALLWORK_MASK looks odd.

It's checked with testw so it must be < 16.