2006-08-03 10:24:40

by Chuck Ebbert

[permalink] [raw]
Subject: [patch] i386: entry.s::error_code is not safe for kprobes

Because code marked unsafe for kprobes jumps directly to
entry.S::error_code, that must be marked unsafe as well.
The easiest way to do that is to move the page fault entry
point to just before error_code and let it inherit the same
section.

Also moved all the ".previous" asm directives for kprobes
sections to column 1 and removed ".text" from them.

Signed-off-by: Chuck Ebbert <[email protected]>

---

x86_64 also has this problem but I wasn't sure whether to move
error_entry into .kprobes.text or convert offendors to the
"paranoid" infrastructure.

arch/i386/kernel/entry.S | 25 +++++++++++++------------
1 files changed, 13 insertions(+), 12 deletions(-)

--- 2.6.18-rc3-nb.orig/arch/i386/kernel/entry.S
+++ 2.6.18-rc3-nb/arch/i386/kernel/entry.S
@@ -587,11 +587,9 @@ ENTRY(name) \
/* The include is where all of the SMP etc. interrupts come from */
#include "entry_arch.h"

-ENTRY(divide_error)
- RING0_INT_FRAME
- pushl $0 # no error code
- CFI_ADJUST_CFA_OFFSET 4
- pushl $do_divide_error
+KPROBE_ENTRY(page_fault)
+ RING0_EC_FRAME
+ pushl $do_page_fault
CFI_ADJUST_CFA_OFFSET 4
ALIGN
error_code:
@@ -641,6 +639,7 @@ error_code:
call *%edi
jmp ret_from_exception
CFI_ENDPROC
+.previous

ENTRY(coprocessor_error)
RING0_INT_FRAME
@@ -716,7 +715,8 @@ debug_stack_correct:
call do_debug
jmp ret_from_exception
CFI_ENDPROC
- .previous .text
+.previous
+
/*
* NMI is doubly nasty. It can happen _while_ we're handling
* a debug fault, and the debug fault hasn't yet been able to
@@ -812,7 +812,7 @@ KPROBE_ENTRY(int3)
call do_int3
jmp ret_from_exception
CFI_ENDPROC
- .previous .text
+.previous

ENTRY(overflow)
RING0_INT_FRAME
@@ -877,7 +877,7 @@ KPROBE_ENTRY(general_protection)
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
- .previous .text
+.previous

ENTRY(alignment_check)
RING0_EC_FRAME
@@ -886,13 +886,14 @@ ENTRY(alignment_check)
jmp error_code
CFI_ENDPROC

-KPROBE_ENTRY(page_fault)
- RING0_EC_FRAME
- pushl $do_page_fault
+ENTRY(divide_error)
+ RING0_INT_FRAME
+ pushl $0 # no error code
+ CFI_ADJUST_CFA_OFFSET 4
+ pushl $do_divide_error
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
- .previous .text

#ifdef CONFIG_X86_MCE
ENTRY(machine_check)
--
Chuck


2006-08-04 05:35:48

by S. P. Prasanna

[permalink] [raw]
Subject: Re: [patch] i386: entry.s::error_code is not safe for kprobes

Hi Chuck,

Thanks for fixing the problem. Patch looks good to me.
I will make those changes for x86_64.

Thanks
Prasanna

On Thu, Aug 03, 2006 at 06:20:19AM -0400, Chuck Ebbert wrote:
> Because code marked unsafe for kprobes jumps directly to
> entry.S::error_code, that must be marked unsafe as well.
> The easiest way to do that is to move the page fault entry
> point to just before error_code and let it inherit the same
> section.
>
> Also moved all the ".previous" asm directives for kprobes
> sections to column 1 and removed ".text" from them.
>
> Signed-off-by: Chuck Ebbert <[email protected]>
>
> ---
>
> x86_64 also has this problem but I wasn't sure whether to move
> error_entry into .kprobes.text or convert offendors to the
> "paranoid" infrastructure.
>
> arch/i386/kernel/entry.S | 25 +++++++++++++------------
> 1 files changed, 13 insertions(+), 12 deletions(-)
>
> --- 2.6.18-rc3-nb.orig/arch/i386/kernel/entry.S
> +++ 2.6.18-rc3-nb/arch/i386/kernel/entry.S
> @@ -587,11 +587,9 @@ ENTRY(name) \
> /* The include is where all of the SMP etc. interrupts come from */
> #include "entry_arch.h"
>
> -ENTRY(divide_error)
> - RING0_INT_FRAME
> - pushl $0 # no error code
> - CFI_ADJUST_CFA_OFFSET 4
> - pushl $do_divide_error
> +KPROBE_ENTRY(page_fault)
> + RING0_EC_FRAME
> + pushl $do_page_fault
> CFI_ADJUST_CFA_OFFSET 4
> ALIGN
> error_code:
> @@ -641,6 +639,7 @@ error_code:
> call *%edi
> jmp ret_from_exception
> CFI_ENDPROC
> +.previous
>
> ENTRY(coprocessor_error)
> RING0_INT_FRAME
> @@ -716,7 +715,8 @@ debug_stack_correct:
> call do_debug
> jmp ret_from_exception
> CFI_ENDPROC
> - .previous .text
> +.previous
> +
> /*
> * NMI is doubly nasty. It can happen _while_ we're handling
> * a debug fault, and the debug fault hasn't yet been able to
> @@ -812,7 +812,7 @@ KPROBE_ENTRY(int3)
> call do_int3
> jmp ret_from_exception
> CFI_ENDPROC
> - .previous .text
> +.previous
>
> ENTRY(overflow)
> RING0_INT_FRAME
> @@ -877,7 +877,7 @@ KPROBE_ENTRY(general_protection)
> CFI_ADJUST_CFA_OFFSET 4
> jmp error_code
> CFI_ENDPROC
> - .previous .text
> +.previous
>
> ENTRY(alignment_check)
> RING0_EC_FRAME
> @@ -886,13 +886,14 @@ ENTRY(alignment_check)
> jmp error_code
> CFI_ENDPROC
>
> -KPROBE_ENTRY(page_fault)
> - RING0_EC_FRAME
> - pushl $do_page_fault
> +ENTRY(divide_error)
> + RING0_INT_FRAME
> + pushl $0 # no error code
> + CFI_ADJUST_CFA_OFFSET 4
> + pushl $do_divide_error
> CFI_ADJUST_CFA_OFFSET 4
> jmp error_code
> CFI_ENDPROC
> - .previous .text
>
> #ifdef CONFIG_X86_MCE
> ENTRY(machine_check)
> --
> Chuck

--
S.P. Prasanna
Linux Technology Center
India Software Labs, IBM Bangalore
Email: [email protected]
Ph: 91-80-41776329

2006-08-04 06:09:23

by Andi Kleen

[permalink] [raw]
Subject: Re: [patch] i386: entry.s::error_code is not safe for kprobes

On Thursday 03 August 2006 12:20, Chuck Ebbert wrote:
> Because code marked unsafe for kprobes jumps directly to
> entry.S::error_code, that must be marked unsafe as well.
> The easiest way to do that is to move the page fault entry
> point to just before error_code and let it inherit the same
> section.
>
> Also moved all the ".previous" asm directives for kprobes
> sections to column 1 and removed ".text" from them.

Ok added thanks

-Andi