x86: fix ESP corruption CPU bug

PATCH In review Stas Sergeev 2 participants 3 messages Jan 3, 2005 – Jan 4, 2005

lore

Share & tools

lore

Stas Sergeev

author

2005-01-03 23:45 UTC #0 lore raw

PATCH 1/1 x86: fix ESP corruption CPU bug

Hi Andrew.

Attached patch works around the corruption of the high word of the ESP register, which is the official bug of x86 CPUs. The bug triggers only when the one is using the 16bit stack segment.
Patch helps running many apps under dosemu, and, according to the comments found in Wine sources, also helps Wine.

The patch defines the per-CPU 16bit stacks, and every time the process that uses 16bit stack, returns to the userspace, we switch to our 16bit stack and preload the high word of ESP.
This also closes the "informational leak", which is that the user process is not supposed to know the kernel's ESP value.

Can this please be applied?

Acked-by Linus Torvalds <[email protected]>

|Signed-off-by: Stas Sergeev <[email protected]>|

arch/i386/kernel/cpu/common.c +12 -0

diff -urN linux-2.6.10-mm1/arch/i386/kernel/cpu/common.c linux-2.6.10-mm1-stk/arch/i386/kernel/cpu/common.c
@@ -16,6 +16,10 @@
 DEFINE_PER_CPU(struct desc_struct, cpu_gdt_table[GDT_ENTRIES]);
 EXPORT_PER_CPU_SYMBOL(cpu_gdt_table);
 
+unsigned char cpu_16bit_stack[CPU_16BIT_STACK_SIZE];
+DEFINE_PER_CPU(unsigned char, cpu_16bit_stack[CPU_16BIT_STACK_SIZE]);
+EXPORT_PER_CPU_SYMBOL(cpu_16bit_stack);
+
 static int cachesize_override __initdata = -1;
 static int disable_x86_fxsr __initdata = 0;
 static int disable_x86_serial_nr __initdata = 1;

@@ -505,6 +509,7 @@
 	int cpu = smp_processor_id();
 	struct tss_struct * t = &per_cpu(init_tss, cpu);
 	struct thread_struct *thread = &current->thread;
+	__u32 stk16_off = (__u32)&per_cpu(cpu_16bit_stack, cpu);
 
 	if (test_and_set_bit(cpu, &cpu_initialized)) {
 		printk(KERN_WARNING "CPU#%d already initialized!\n", cpu);

@@ -527,6 +532,13 @@
 	 */
 	memcpy(&per_cpu(cpu_gdt_table, cpu), cpu_gdt_table,
 	       GDT_SIZE);
+
+	/* Set up GDT entry for 16bit stack */
+	*(__u64 *)&(per_cpu(cpu_gdt_table, cpu)[GDT_ENTRY_ESPFIX_SS]) |=
+		((((__u64)stk16_off) << 16) & 0x000000ffffff0000ULL) |
+		((((__u64)stk16_off) << 32) & 0xff00000000000000ULL) |
+		(CPU_16BIT_STACK_SIZE - 1);
+
 	cpu_gdt_descr[cpu].size = GDT_SIZE - 1;
 	cpu_gdt_descr[cpu].address =
 	    (unsigned long)&per_cpu(cpu_gdt_table, cpu);

arch/i386/kernel/entry.S +106 ~~-30~~

216 lines not on this page · raw message

arch/i386/kernel/head.S +1 -1

diff -urN linux-2.6.10-mm1/arch/i386/kernel/head.S linux-2.6.10-mm1-stk/arch/i386/kernel/head.S
@@ -512,7 +512,7 @@
 	.quad 0x00009a0000000000	/* 0xc0 APM CS 16 code (16 bit) */
 	.quad 0x0040920000000000	/* 0xc8 APM DS    data */
 
-	.quad 0x0000000000000000	/* 0xd0 - unused */
+	.quad 0x0000920000000000	/* 0xd0 - ESPFIX 16-bit SS */
 	.quad 0x0000000000000000	/* 0xd8 - unused */
 	.quad 0x0000000000000000	/* 0xe0 - unused */
 	.quad 0x0000000000000000	/* 0xe8 - unused */

arch/i386/kernel/traps.c +45 -0

51 lines not on this page · raw message

include/asm-i386/desc.h +5 -0

diff -urN linux-2.6.10-mm1/include/asm-i386/desc.h linux-2.6.10-mm1-stk/include/asm-i386/desc.h
@@ -4,6 +4,8 @@
 #include <asm/ldt.h>
 #include <asm/segment.h>
 
+#define CPU_16BIT_STACK_SIZE 1024
+
 #ifndef __ASSEMBLY__
 
 #include <linux/preempt.h>

@@ -15,6 +17,9 @@
 extern struct desc_struct cpu_gdt_table[GDT_ENTRIES];
 DECLARE_PER_CPU(struct desc_struct, cpu_gdt_table[GDT_ENTRIES]);
 
+extern unsigned char cpu_16bit_stack[CPU_16BIT_STACK_SIZE];
+DECLARE_PER_CPU(unsigned char, cpu_16bit_stack[CPU_16BIT_STACK_SIZE]);
+
 struct Xgt_desc_struct {
 	unsigned short size;
 	unsigned long address __attribute__((packed));

include/asm-i386/segment.h +4 -1

diff -urN linux-2.6.10-mm1/include/asm-i386/segment.h linux-2.6.10-mm1-stk/include/asm-i386/segment.h
@@ -38,7 +38,7 @@
  *  24 - APM BIOS support
  *  25 - APM BIOS support 
  *
- *  26 - unused
+ *  26 - ESPFIX small SS
  *  27 - unused
  *  28 - unused
  *  29 - unused

@@ -71,6 +71,9 @@
 #define GDT_ENTRY_PNPBIOS_BASE		(GDT_ENTRY_KERNEL_BASE + 6)
 #define GDT_ENTRY_APMBIOS_BASE		(GDT_ENTRY_KERNEL_BASE + 11)
 
+#define GDT_ENTRY_ESPFIX_SS		(GDT_ENTRY_KERNEL_BASE + 14)
+#define __ESPFIX_SS (GDT_ENTRY_ESPFIX_SS * 8)
+
 #define GDT_ENTRY_DOUBLEFAULT_TSS	31
 
 /*

Linus Torvalds

2005-01-04 00:05 UTC #1 lore raw

Quoted 2 lines · Stas Sergeev

Can this please be applied?

Please don't do it like this - you made the patch now depend on the ugliest code in the universe, namely that horribly crappy kgdb-ga sh*t ("Don't hold back, Linus, tell us how you really feel").

The 16-bit stack code may not be the prettiest either, but it doesn't hold a candle to the asm-crap that is entry.S after kgdb-ga.

"resume_kernelX"? What crud.

Linus

Stas Sergeev

author

2005-01-04 01:58 UTC #2 lore raw

Hi Linus.

Quoted 1 line · Linus Torvalds

Please don't do it like this - you made the patch now depend on the ugliest code in the universe, namely that horribly crappy kgdb-ga sh*t

I didn't do that. I just re-targeted the patch to -mm tree and it clashed with the kgdb-ga patch. And somehow it happened that a few lines I would have to insert myself otherwise anyway, appeared to be already there for me to re-use. I wouldn't call that a dependancy. Only 3 lines are re-used in fact.

Quoted 2 lines

The 16-bit stack code may not be the prettiest either, but it doesn't hold a candle to the asm-crap that is entry.S after kgdb-ga. From what I can see, kgdb-ga have only

3 small hunks in entry.S, so the crap is probably very dense there.
Clashing into one of these hunks looks unavoidable for my needs.

Quoted 1 line

"resume_kernelX"? What crud.

Does "restore_nocheck" sound better? Yes, maybe, but then I don't see the way to provide my patch for -mm. So the attached one is for plain 2.6.10. I don't know how Andrew can apply it, so maybe you will?

Signed-off-by Stas Sergeev <[email protected]>