Skip to contentSkip to conversation

sanitize TLS API

PATCH In review Christoph Hellwig 4 participants 5 messages Jul 30, 2002 – Jul 31, 2002

lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 sanitize TLS API

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.

arch/i386/kernel/entry.S +1 -0
@@ -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
arch/i386/kernel/process.c +19 -28

60 lines not on this page · raw message

include/asm-i386/desc.h +0 -5
@@ -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;
include/asm-i386/unistd.h +2 -0
@@ -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> */
#1 lore raw
Quoted 6 lines · 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.

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

Ingo

#2 lore raw
Quoted 6 lines · Ingo Molnar
Quoted 3 lines · Christoph Hellwig

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

Quoted 6 lines
Quoted 3 lines

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

#3 lore raw
Quoted 4 lines · Jakub Jelinek

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

arch/i386/kernel/process.c renamed from arch/i386/kernel/process.c.orig +0 -11
@@ -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;
include/asm-i386/desc.h renamed from include/asm-i386/desc.h.orig +1 -2
@@ -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)
 {
include/asm-i386/unistd.h renamed from include/asm-i386/unistd.h.orig +1 -0
@@ -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> */
#4 lore raw
In reply to Jakub Jelinek
Quoted 4 lines · Jakub Jelinek

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