[PATCH]x86_64 debug_stack nested patch

[PATCH]x86_64 debug_stack nested patch

[bibo,mao]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

hi,
In x86_64 platform, INT1 and INT3 trap stack is IST stack called 
DEBUG_STACK, when INT1/INT3 trap happens, system will switch to 
DEBUG_STACK by hardware. Current DEBUG_STACK size is 4K, when int1/int3 
trap happens, kernel will minus current DEBUG_STACK IST value by 4k. But 
if int3/int1 trap is nested, it will destroy other vector's IST stack.
This patch modifies this, it sets DEBUG_STACK size as 8K and allows two 
level of nested int1/int3 trap.
Kprobe DEBUG_STACK may be nested, because kprobe hanlder may be probed 
by other kprobes. This patch is against 2.6.17-rc3.

Signed-Off-By: bibo, mao <bibo.mao@intel.com>

Thanks
bibo,mao

[-- Attachment #2: DEBUG_STACK_NEST.patch --]
[-- Type: text/x-patch, Size: 2213 bytes --]

diff -Nruap 2.6.17-rc3.org/arch/x86_64/kernel/traps.c 2.6.17-rc3/arch/x86_64/kernel/traps.c
--- 2.6.17-rc3.org/arch/x86_64/kernel/traps.c	2006-05-10 12:07:30.000000000 +0800
+++ 2.6.17-rc3/arch/x86_64/kernel/traps.c	2006-05-10 12:18:53.000000000 +0800
@@ -141,50 +141,24 @@ static unsigned long *in_exception_stack
 		[DOUBLEFAULT_STACK - 1] = "#DF",
 		[STACKFAULT_STACK - 1] = "#SS",
 		[MCE_STACK - 1] = "#MC",
-#if DEBUG_STKSZ > EXCEPTION_STKSZ
-		[N_EXCEPTION_STACKS ... N_EXCEPTION_STACKS + DEBUG_STKSZ / EXCEPTION_STKSZ - 2] = "#DB[?]"
-#endif
 	};
-	unsigned k;
+	unsigned stack_size, end, k;
 
 	for (k = 0; k < N_EXCEPTION_STACKS; k++) {
-		unsigned long end;
-
-		switch (k + 1) {
-#if DEBUG_STKSZ > EXCEPTION_STKSZ
-		case DEBUG_STACK:
-			end = cpu_pda(cpu)->debugstack + DEBUG_STKSZ;
-			break;
-#endif
-		default:
-			end = per_cpu(init_tss, cpu).ist[k];
-			break;
-		}
+		end = per_cpu(init_tss, cpu).ist[k];
 		if (stack >= end)
 			continue;
-		if (stack >= end - EXCEPTION_STKSZ) {
+		if (k == (DEBUG_STACK - 1))
+			stack_size = DEBUG_STKSZ;
+		else stack_size = EXCEPTION_STKSZ;
+
+		if (stack >= end - stack_size) {
 			if (*usedp & (1U << k))
 				break;
 			*usedp |= 1U << k;
 			*idp = ids[k];
 			return (unsigned long *)end;
 		}
-#if DEBUG_STKSZ > EXCEPTION_STKSZ
-		if (k == DEBUG_STACK - 1 && stack >= end - DEBUG_STKSZ) {
-			unsigned j = N_EXCEPTION_STACKS - 1;
-
-			do {
-				++j;
-				end -= EXCEPTION_STKSZ;
-				ids[j][4] = '1' + (j - N_EXCEPTION_STACKS);
-			} while (stack < end - EXCEPTION_STKSZ);
-			if (*usedp & (1U << j))
-				break;
-			*usedp |= 1U << j;
-			*idp = ids[j];
-			return (unsigned long *)end;
-		}
-#endif
 	}
 	return NULL;
 }
diff -Nruap 2.6.17-rc3.org/include/asm-x86_64/page.h 2.6.17-rc3/include/asm-x86_64/page.h
--- 2.6.17-rc3.org/include/asm-x86_64/page.h	2006-05-10 12:07:18.000000000 +0800
+++ 2.6.17-rc3/include/asm-x86_64/page.h	2006-05-10 12:19:24.000000000 +0800
@@ -20,7 +20,7 @@
 #define EXCEPTION_STACK_ORDER 0
 #define EXCEPTION_STKSZ (PAGE_SIZE << EXCEPTION_STACK_ORDER)
 
-#define DEBUG_STACK_ORDER EXCEPTION_STACK_ORDER
+#define DEBUG_STACK_ORDER 1
 #define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
 
 #define IRQSTACK_ORDER 2

Re: [PATCH]x86_64 debug_stack nested patch

[Jan Beulich]

Would you mind explaining why you
- needed to replace the handling of the DEBUG_STKSZ > EXCEPTION_STKSZ in in_exception_stack()?
- used a hard coded 1 instead of (EXCEPTION_STACK_ORDER + 1) for defining DEBUG_STACK_ORDER?

Thanks, Jan

>>> "bibo,mao" <bibo.mao@intel.com> 10.05.06 08:45 >>>
hi,
In x86_64 platform, INT1 and INT3 trap stack is IST stack called 
DEBUG_STACK, when INT1/INT3 trap happens, system will switch to 
DEBUG_STACK by hardware. Current DEBUG_STACK size is 4K, when int1/int3 
trap happens, kernel will minus current DEBUG_STACK IST value by 4k. But 
if int3/int1 trap is nested, it will destroy other vector's IST stack.
This patch modifies this, it sets DEBUG_STACK size as 8K and allows two 
level of nested int1/int3 trap.
Kprobe DEBUG_STACK may be nested, because kprobe hanlder may be probed 
by other kprobes. This patch is against 2.6.17-rc3.

Signed-Off-By: bibo, mao <bibo.mao@intel.com>

Thanks
bibo,mao

Re: [PATCH]x86_64 debug_stack nested patch

[bibo,mao]

ok, EXCEPTION_STACK_ORDER + 1 is better for me.
in_exception_stack() function is to judge which IST stack by parameter 
stack value, if DEBUG_STKSZ value is set as 8K. The original function 
can not judge whether it is within DEBUG_STACK space.

Thanks
bibo,mao

Jan Beulich wrote:
> Would you mind explaining why you
> - needed to replace the handling of the DEBUG_STKSZ > EXCEPTION_STKSZ in in_exception_stack()?
> - used a hard coded 1 instead of (EXCEPTION_STACK_ORDER + 1) for defining DEBUG_STACK_ORDER?
> 
> Thanks, Jan
> 
>>>> "bibo,mao" <bibo.mao@intel.com> 10.05.06 08:45 >>>
> hi,
> In x86_64 platform, INT1 and INT3 trap stack is IST stack called 
> DEBUG_STACK, when INT1/INT3 trap happens, system will switch to 
> DEBUG_STACK by hardware. Current DEBUG_STACK size is 4K, when int1/int3 
> trap happens, kernel will minus current DEBUG_STACK IST value by 4k. But 
> if int3/int1 trap is nested, it will destroy other vector's IST stack.
> This patch modifies this, it sets DEBUG_STACK size as 8K and allows two 
> level of nested int1/int3 trap.
> Kprobe DEBUG_STACK may be nested, because kprobe hanlder may be probed 
> by other kprobes. This patch is against 2.6.17-rc3.
> 
> Signed-Off-By: bibo, mao <bibo.mao@intel.com>
> 
> Thanks
> bibo,mao
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH]x86_64 debug_stack nested patch

[Jan Beulich]

>in_exception_stack() function is to judge which IST stack by parameter 
>stack value, if DEBUG_STKSZ value is set as 8K. The original function 
>can not judge whether it is within DEBUG_STACK space.

I rather think that the new code can't work properly. Since the pointer in the TSS gets decreased while the handler is
running, using that value is not going to tell you the end of the stack, but you'd rather get the end of the stack the
next (nested) invocation of the handler would use. Further, treating the entire DEBUG_STKSZ range as a single piece is
wrong, too, because it is not being used as a contiguous stack (but rather as 2 stacks EXCEPTION_STKSZ in size); the new
code shouldn't be able to properly deal with nested invocations because of this.

Jan

Re: [PATCH]x86_64 debug_stack nested patch

[bibo,mao]

yes, I am wrong. And I will modify this. And then only need define 
DEBUG_STACK_ORDER as (EXCEPTION_STACK_ORDER + 1)

thanks
bibo,mao

Jan Beulich wrote:
>> in_exception_stack() function is to judge which IST stack by parameter 
>> stack value, if DEBUG_STKSZ value is set as 8K. The original function 
>> can not judge whether it is within DEBUG_STACK space.
> 
> I rather think that the new code can't work properly. Since the pointer in the TSS gets decreased while the handler is
> running, using that value is not going to tell you the end of the stack, but you'd rather get the end of the stack the
> next (nested) invocation of the handler would use. Further, treating the entire DEBUG_STKSZ range as a single piece is
> wrong, too, because it is not being used as a contiguous stack (but rather as 2 stacks EXCEPTION_STKSZ in size); the new
> code shouldn't be able to properly deal with nested invocations because of this.
> 
> Jan
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH]x86_64 debug_stack nested patch

[Andi Kleen]

On Wednesday 10 May 2006 10:26, bibo,mao wrote:
> yes, I am wrong. And I will modify this. And then only need define 
> DEBUG_STACK_ORDER as (EXCEPTION_STACK_ORDER + 1)

If the kprobes code really wants to nest (in my opinion it's a kprobes
bug) you should reduce its stack to 2K or so. Don't want to waste 
space for stupid code like this.

-Andi