Re: Sysenter crash with Nested Task Bit set

[Linus Torvalds]

On Sun, 17 Sep 2006, Andrew Morton wrote:
> > 
> > I'll take your word that it crashes.
> 
> It doesn't for me - I get a segfault.
> 
> That's on a PIII.  Are recenter CPUs different in this regard?

No, I don't think they are, but I think your exact machine matters. For 
example, an SMP machine will behave differently, because NT will be set 
only on one CPU.

I _think_ that the problem is that once NT gets set in kernel space (and 
sysenter is the only way to do that, since the normal fault paths will 
clear NT), it stays set, even across a context switch. Nothing ever 
reloads a kernel eflags, because we "trust" eflags in kernel space.

So Chuck's patch doesn't really fix it - because a preemptible kernel 
might context switch long before the original task that set eflags, and 
thus NT in eflags might migrate to somebody else, and while we'll 
eventually take the "invalid TSS" error and then Chuck's patch will cause 
us to clear it, we might be taking the exception for the wrong task (and 
thus kill the wrong guy).

And sysenter really is very special because of the weak trap semantics. 
Damn. We could either fix it in the sysenter code-path, or in the 
task-switch one, and both of them are timing-critical, but task switching 
perhaps just a tad less so.

If we fix it in the task-switch code, we shouldn't need any other changes 
(ie Chuck's change is unnecessary too), because then the process that sets 
NT will happily die (with NT set), but switch away to something else and 
nobody else will be affected.

So if I'm right, then this patch _should_ fix it. UNTESTED (and the 
"ref_from_fork" special case doesn't clear NT, so it's strictly incompete, 
but maybe somebody can test this?)

Hmm? Ingo? Comments?

Andi? I don't know if x86-64 honors NT in 64-bit mode, but if it does, it 
needs something similar (assuming this works).

		Linus

---
diff --git a/include/asm-i386/system.h b/include/asm-i386/system.h
index 49928eb..f6e4260 100644
--- a/include/asm-i386/system.h
+++ b/include/asm-i386/system.h
@@ -13,7 +13,8 @@ extern struct task_struct * FASTCALL(__s
 
 #define switch_to(prev,next,last) do {					\
 	unsigned long esi,edi;						\
-	asm volatile("pushl %%ebp\n\t"					\
+	asm volatile("pushfl\n\t"		/* Save flags */	\
+		     "pushl %%ebp\n\t"					\
 		     "movl %%esp,%0\n\t"	/* save ESP */		\
 		     "movl %5,%%esp\n\t"	/* restore ESP */	\
 		     "movl $1f,%1\n\t"		/* save EIP */		\
@@ -21,6 +22,7 @@ #define switch_to(prev,next,last) do {		
 		     "jmp __switch_to\n"				\
 		     "1:\t"						\
 		     "popl %%ebp\n\t"					\
+		     "popfl"						\
 		     :"=m" (prev->thread.esp),"=m" (prev->thread.eip),	\
 		      "=a" (last),"=S" (esi),"=D" (edi)			\
 		     :"m" (next->thread.esp),"m" (next->thread.eip),	\
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/