2006-05-10 09:26:22

by bibo,mao

[permalink] [raw]
Subject: [PATCH]x86_64 debug_stack nested patch (again)

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. Thanks jbeulich for pointing out error in the first patch.

Signed-Off-By: bibo, mao <[email protected]>

--- 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 (EXCEPTION_STACK_ORDER + 1)
#define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)

#define IRQSTACK_ORDER 2


Thanks
bibo,mao


2006-05-11 11:20:06

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH]x86_64 debug_stack nested patch (again)

"bibo,mao" <[email protected]> wrote:
>
> 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. Thanks jbeulich for pointing out error in the first patch.
>
> Signed-Off-By: bibo, mao <[email protected]>
>
> --- 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 (EXCEPTION_STACK_ORDER + 1)
> #define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
>
> #define IRQSTACK_ORDER 2

So.... why not do it this way?



--- devel/include/asm-x86_64/page.h~x86_64-kprobes-debug_stack-nesting-fix 2006-05-11 04:15:12.000000000 -0700
+++ devel-akpm/include/asm-x86_64/page.h 2006-05-11 04:16:07.000000000 -0700
@@ -20,7 +20,15 @@
#define EXCEPTION_STACK_ORDER 0
#define EXCEPTION_STKSZ (PAGE_SIZE << EXCEPTION_STACK_ORDER)

+#ifdef CONFIG_KPROBES
+/*
+ * kprobes uses an 8k stack because int1/int3 exceptions can nest
+ */
+#define DEBUG_STACK_ORDER (EXCEPTION_STACK_ORDER + 1)
+#else
#define DEBUG_STACK_ORDER EXCEPTION_STACK_ORDER
+#endif
+
#define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)

#define IRQSTACK_ORDER 2
_

2006-05-11 11:28:51

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH]x86_64 debug_stack nested patch (again)

On Thursday 11 May 2006 13:17, Andrew Morton wrote:
> "bibo,mao" <[email protected]> wrote:
> >
> > 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. Thanks jbeulich for pointing out error in the first patch.
> >
> > Signed-Off-By: bibo, mao <[email protected]>
> >
> > --- 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 (EXCEPTION_STACK_ORDER + 1)
> > #define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
> >
> > #define IRQSTACK_ORDER 2
>
> So.... why not do it this way?

Last time we discussed this I was told it could nest upto 3 or 4 times
So that still wouldn't work.

If anything they should decrease the int3/debug stack to 2K, then 8K
might be enough.

Or even better would be to fix kprobes to not do that.

I think paranoidentry would need to be fixed for that too.

-Andi

2006-05-16 09:43:03

by bibo,mao

[permalink] [raw]
Subject: Re: [PATCH]x86_64 debug_stack nested patch (again)

Sorry for late reply, interrupt is disabled when int3/int1 trap happens
and NMI is not permitted for kprobe in x86_64 platform,nest level of kprobe
is 2 at most. So that will work well if DEBUG_STACK is set to 8K when CONFIG_KPROBE
option is set.

Thanks
bibo,mao

Andi Kleen wrote:
> On Thursday 11 May 2006 13:17, Andrew Morton wrote:
>> "bibo,mao" <[email protected]> wrote:
>>> 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. Thanks jbeulich for pointing out error in the first patch.
>>>
>>> Signed-Off-By: bibo, mao <[email protected]>
>>>
>>> --- 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 (EXCEPTION_STACK_ORDER + 1)
>>> #define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
>>>
>>> #define IRQSTACK_ORDER 2
>> So.... why not do it this way?
>
> Last time we discussed this I was told it could nest upto 3 or 4 times
> So that still wouldn't work.
>
> If anything they should decrease the int3/debug stack to 2K, then 8K
> might be enough.
>
> Or even better would be to fix kprobes to not do that.
>
> I think paranoidentry would need to be fixed for that too.
>
> -Andi
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>