[PATCH] sanitize TLS API

[PATCH] sanitize TLS API

[Christoph Hellwig]

Currently sys_set_thread_area has a magic flags argument that might
change it's behaivour completly.

Split out the TLS_FLAG_CLEAR case that has nothing in common with the
rest into it's own syscall, sys_clear_thread_area and change the
second argument to int writable.


--- 1.39/arch/i386/kernel/entry.S	Tue Jul 30 00:08:08 2002
+++ edited/arch/i386/kernel/entry.S	Tue Jul 30 11:46:01 2002
@@ -753,6 +753,7 @@
 	.long sys_sched_setaffinity
 	.long sys_sched_getaffinity
 	.long sys_set_thread_area
+	.long sys_clear_thread_area
 
 	.rept NR_syscalls-(.-sys_call_table)/4
 		.long sys_ni_syscall
--- 1.29/arch/i386/kernel/process.c	Mon Jul 29 04:07:12 2002
+++ edited/arch/i386/kernel/process.c	Tue Jul 30 12:34:21 2002
@@ -831,38 +831,15 @@
 /*
  * Set the Thread-Local Storage area:
  */
-asmlinkage int sys_set_thread_area(unsigned long base, unsigned long flags)
+asmlinkage int sys_set_thread_area(unsigned long base, int writable)
 {
 	struct thread_struct *t = &current->thread;
-	int writable = 0;
-	int cpu;
+	int cpu = get_cpu();
 
-	/* do not allow unused flags */
-	if (flags & ~TLS_FLAGS_MASK)
-		return -EINVAL;
-
-	/*
-	 * Clear the TLS?
-	 */
-	if (flags & TLS_FLAG_CLEAR) {
-		cpu = get_cpu();
-        	t->tls_desc.a = t->tls_desc.b = 0;
-		load_TLS_desc(t, cpu);
-		put_cpu();
-		return 0;
-	}
-
-	if (flags & TLS_FLAG_WRITABLE)
-		writable = 1;
-
-	/*
-	 * We must not get preempted while modifying the TLS.
-	 */
-	cpu = get_cpu();
+	writable = !!writable; /* must be one or zero */
 
-        t->tls_desc.a = ((base & 0x0000ffff) << 16) | 0xffff;
-
-        t->tls_desc.b = (base & 0xff000000) | ((base & 0x00ff0000) >> 16) |
+	t->tls_desc.a = ((base & 0x0000ffff) << 16) | 0xffff;
+	t->tls_desc.b = (base & 0xff000000) | ((base & 0x00ff0000) >> 16) |
 				0xf0000 | (writable << 9) | (1 << 15) |
 					(1 << 22) | (1 << 23) | 0x7000;
 
@@ -872,3 +849,17 @@
 	return TLS_ENTRY*8 + 3;
 }
 
+
+/*
+ * Clear the Thread-Local Storage area:
+ */
+asmlinkage void sys_clear_thread_area(void)
+{
+	struct thread_struct *t = &current->thread;
+	int cpu = get_cpu();
+
+	t->tls_desc.a = t->tls_desc.b = 0;
+	load_TLS_desc(t, cpu);
+
+	put_cpu();
+}
--- 1.7/include/asm-i386/desc.h	Mon Jul 29 04:07:52 2002
+++ edited/include/asm-i386/desc.h	Tue Jul 30 11:48:40 2002
@@ -86,11 +86,6 @@
 	_set_tssldt_desc(&cpu_gdt_table[cpu][LDT_ENTRY], (int)addr, ((size << 3)-1), 0x82);
 }
 
-#define TLS_FLAGS_MASK			0x00000003
-
-#define TLS_FLAG_WRITABLE		0x00000001
-#define TLS_FLAG_CLEAR			0x00000002
-
 static inline void load_TLS_desc(struct thread_struct *t, unsigned int cpu)
 {
 	cpu_gdt_table[cpu][TLS_ENTRY] = t->tls_desc;
--- 1.11/include/asm-i386/unistd.h	Tue Jul 30 00:08:09 2002
+++ edited/include/asm-i386/unistd.h	Tue Jul 30 11:47:20 2002
@@ -247,6 +247,8 @@
 #define __NR_futex		240
 #define __NR_sched_setaffinity	241
 #define __NR_sched_getaffinity	242
+#define __NR_set_thread_area	243
+#define __NR_clear_thread_area	244
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 

Re: [PATCH] sanitize TLS API

[Ingo Molnar]

On Tue, 30 Jul 2002, Christoph Hellwig wrote:

> Currently sys_set_thread_area has a magic flags argument that might
> change it's behaivour completly.
> 
> Split out the TLS_FLAG_CLEAR case that has nothing in common with the
> rest into it's own syscall, sys_clear_thread_area and change the second
> argument to int writable.

i did not feel like wasting two syscall slots for this, but the cleanups
look fine to me otherwise.

	Ingo

Re: [PATCH] sanitize TLS API

[Jakub Jelinek]

On Tue, Jul 30, 2002 at 09:00:09PM +0200, Ingo Molnar wrote:
> 
> On Tue, 30 Jul 2002, Christoph Hellwig wrote:
> 
> > Currently sys_set_thread_area has a magic flags argument that might
> > change it's behaivour completly.
> > 
> > Split out the TLS_FLAG_CLEAR case that has nothing in common with the
> > rest into it's own syscall, sys_clear_thread_area and change the second
> > argument to int writable.
> 
> i did not feel like wasting two syscall slots for this, but the cleanups
> look fine to me otherwise.

Actually, is the clear operation really necessary?
IMHO the best clear is movw $0x03, %gs, then all accesses through %gs will
trap. Calling set_thread_area (0, 1); will result in 0xb segment
acting exactly like %ds or %es.

	Jakub

Re: [PATCH] sanitize TLS API

[Ingo Molnar]

On Tue, 30 Jul 2002, Jakub Jelinek wrote:

> Actually, is the clear operation really necessary? IMHO the best clear
> is movw $0x03, %gs, then all accesses through %gs will trap. Calling
> set_thread_area (0, 1); will result in 0xb segment acting exactly like
> %ds or %es.

indeed. I've attached a (tested) patch against BK-curr that removes just
the clear operation. I've left the flags mask and the writable flag just
so that we have the option to introduce extensions without breaking the
ABI.

	Ingo

--- linux/arch/i386/kernel/process.c.orig	Tue Jul 30 22:13:43 2002
+++ linux/arch/i386/kernel/process.c	Tue Jul 30 22:13:52 2002
@@ -851,17 +851,6 @@
 	if (flags & ~TLS_FLAGS_MASK)
 		return -EINVAL;
 
-	/*
-	 * Clear the TLS?
-	 */
-	if (flags & TLS_FLAG_CLEAR) {
-		cpu = get_cpu();
-        	t->tls_desc.a = t->tls_desc.b = 0;
-		load_TLS_desc(t, cpu);
-		put_cpu();
-		return 0;
-	}
-
 	if (flags & TLS_FLAG_WRITABLE)
 		writable = 1;
 
--- linux/include/asm-i386/desc.h.orig	Tue Jul 30 22:14:10 2002
+++ linux/include/asm-i386/desc.h	Tue Jul 30 22:14:27 2002
@@ -86,10 +86,9 @@
 	_set_tssldt_desc(&cpu_gdt_table[cpu][LDT_ENTRY], (int)addr, ((size << 3)-1), 0x82);
 }
 
-#define TLS_FLAGS_MASK			0x00000003
+#define TLS_FLAGS_MASK			0x00000001
 
 #define TLS_FLAG_WRITABLE		0x00000001
-#define TLS_FLAG_CLEAR			0x00000002
 
 static inline void load_TLS_desc(struct thread_struct *t, unsigned int cpu)
 {
--- linux/include/asm-i386/unistd.h.orig	Tue Jul 30 22:14:40 2002
+++ linux/include/asm-i386/unistd.h	Tue Jul 30 22:15:05 2002
@@ -247,6 +247,7 @@
 #define __NR_futex		240
 #define __NR_sched_setaffinity	241
 #define __NR_sched_getaffinity	242
+#define __NR_set_thread_area	243
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 

Re: [PATCH] sanitize TLS API

[Andi Kleen]

Jakub Jelinek <jakub@redhat.com> writes:

> Actually, is the clear operation really necessary?
> IMHO the best clear is movw $0x03, %gs, then all accesses through %gs will
> trap. Calling set_thread_area (0, 1); will result in 0xb segment
> acting exactly like %ds or %es.

At least on x86-64 it is useful to have a clear operation, because setting the
thread descriptors adds some cost to the context switch for various reasons. 
This way processes you could disable it again when they don't need it anymore.

-Andi