[v3 PATCH 1/3] powerpc/kprobe: introduce a new thread flag

[v3 PATCH 1/3] powerpc/kprobe: introduce a new thread flag

[Tiejun Chen]

We need to add a new thread flag, TIF_EMULATE_STACK_STORE,
for emulating stack store operation while exiting exception.

Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
 arch/powerpc/include/asm/thread_info.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index bcebc75..45d098c 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -110,6 +110,8 @@ static inline struct thread_info *current_thread_info(void)
 #define TIF_NOERROR		12	/* Force successful syscall return */
 #define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
 #define TIF_SYSCALL_TRACEPOINT	15	/* syscall tracepoint instrumentation */
+#define	TIF_EMULATE_STACK_STORE	17	/* Is an instruction emulation
+						for stack store? */
 
 /* as above, but as bit values */
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
@@ -126,6 +128,7 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_NOERROR		(1<<TIF_NOERROR)
 #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
 #define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
+#define	_TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
 #define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT)
 
-- 
1.5.6

[v3 PATCH 2/3] ppc32/kprobe: complete kprobe and migrate exception frame

[Tiejun Chen]

We can't emulate stwu since that may corrupt current exception stack.
So we will have to do real store operation in the exception return code.

Firstly we'll allocate a trampoline exception frame below the kprobed
function stack and copy the current exception frame to the trampoline.
Then we can do this real store operation to implement 'stwu', and reroute
the trampoline frame to r1 to complete this exception migration.

Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
 arch/powerpc/kernel/entry_32.S |   43 +++++++++++++++++++++++++++++++++------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index ba3aeb4..e7eefdf 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -829,19 +829,50 @@ restore_user:
 	bnel-	load_dbcr0
 #endif
 
-#ifdef CONFIG_PREEMPT
 	b	restore
 
 /* N.B. the only way to get here is from the beq following ret_from_except. */
 resume_kernel:
-	/* check current_thread_info->preempt_count */
+	/* check current_thread_info, _TIF_EMULATE_STACK_STORE */
 	rlwinm	r9,r1,0,0,(31-THREAD_SHIFT)
-	lwz	r0,TI_PREEMPT(r9)
-	cmpwi	0,r0,0		/* if non-zero, just restore regs and return */
-	bne	restore
 	lwz	r0,TI_FLAGS(r9)
+	andis.	r0,r0,_TIF_EMULATE_STACK_STORE@h
+	beq+	1f
+
+	addi	r8,r1,INT_FRAME_SIZE	/* Get the kprobed function entry */
+
+	lwz	r3,GPR1(r1)
+	subi	r3,r3,INT_FRAME_SIZE	/* dst: Allocate a trampoline exception frame */
+	mr	r4,r1			/* src:  current exception frame */
+	li	r5,INT_FRAME_SIZE	/* size: INT_FRAME_SIZE */
+	mr	r1,r3			/* Reroute the trampoline frame to r1 */
+	bl	memcpy			/* Copy from the original to the trampoline */
+
+	/* Do real store operation to complete stwu */
+	lwz	r5,GPR1(r1)
+	stw	r8,0(r5)
+
+	/* Clear _TIF_EMULATE_STACK_STORE flag */
+	rlwinm	r9,r1,0,0,(31-THREAD_SHIFT)
+	lis	r11,_TIF_EMULATE_STACK_STORE@h
+	addi	r5,r9,TI_FLAGS
+0:	lwarx	r8,0,r5
+	andc	r8,r8,r11
+#ifdef CONFIG_IBM405_ERR77
+	dcbt	0,r5
+#endif
+	stwcx.	r8,0,r5
+	bne-	0b
+1:
+
+#ifdef CONFIG_PREEMPT
+	/* check current_thread_info->preempt_count */
+	lwz	r8,TI_PREEMPT(r9)
+	cmpwi	0,r8,0		/* if non-zero, just restore regs and return */
+	bne	restore
 	andi.	r0,r0,_TIF_NEED_RESCHED
 	beq+	restore
+	lwz	r3,_MSR(r1)	
 	andi.	r0,r3,MSR_EE	/* interrupts off? */
 	beq	restore		/* don't schedule if so */
 #ifdef CONFIG_TRACE_IRQFLAGS
@@ -862,8 +893,6 @@ resume_kernel:
 	 */
 	bl	trace_hardirqs_on
 #endif
-#else
-resume_kernel:
 #endif /* CONFIG_PREEMPT */
 
 	/* interrupts are hard-disabled at this point */
-- 
1.5.6

[v3 PATCH 3/3] ppc32/kprobe: don't emulate store when kprobe stwu r1

[Tiejun Chen]

We don't do the real store operation for kprobing 'stwu Rx,(y)R1'
since this may corrupt the exception frame, now we will do this
operation safely in exception return code after migrate current
exception frame below the kprobed function stack.

So we only update gpr[1] here and trigger a thread flag to mask
this.

Note we should make sure if we trigger kernel stack over flow.

Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
 arch/powerpc/lib/sstep.c |   37 +++++++++++++++++++++++++++++++++++--
 1 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 9a52349..a4ce463 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -566,7 +566,7 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
 	unsigned long int ea;
 	unsigned int cr, mb, me, sh;
 	int err;
-	unsigned long old_ra;
+	unsigned long old_ra, val3, r1;
 	long ival;
 
 	opcode = instr >> 26;
@@ -1486,11 +1486,44 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
 		goto ldst_done;
 
 	case 36:	/* stw */
-	case 37:	/* stwu */
 		val = regs->gpr[rd];
 		err = write_mem(val, dform_ea(instr, regs), 4, regs);
 		goto ldst_done;
 
+	case 37:	/* stwu */
+		__asm__ __volatile__("mr %0,1" : "=r" (r1) :);
+
+		val = regs->gpr[rd];
+		val3 = dform_ea(instr, regs);
+		/*
+		 * For PPC32 we always use stwu to change stack point with r1. So
+		 * this emulated store may corrupt the exception frame, now we
+		 * have to provide the exception frame trampoline, which is pushed
+		 * below the kprobed function stack. So we only update gpr[1] but
+		 * don't emulate the real store operation. We will do real store
+		 * operation safely in exception return code by checking this flag.
+		 */
+		if ((ra == 1) && !(regs->msr & MSR_PR) && (val3 >= r1)) {
+			/*
+			 * Check if we will touch kernel sack overflow
+			 */
+			if (r1 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) {
+				printk(KERN_CRIT "Can't kprobe this since Kernel stack overflow.\n");
+				err = -EINVAL;
+				break;
+			}
+
+			/*
+			 * Check if we already set since that means we'll
+			 * lose the previous value.
+			 */
+			WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE));
+			set_thread_flag(TIF_EMULATE_STACK_STORE);
+			err = 0;
+		} else
+			err = write_mem(val, val3, 4, regs);
+		goto ldst_done;
+
 	case 38:	/* stb */
 	case 39:	/* stbu */
 		val = regs->gpr[rd];
-- 
1.5.6

Re: [v3 PATCH 3/3] ppc32/kprobe: don't emulate store when kprobe stwu r1

[tiejun.chen]

On 06/03/2012 01:07 PM, Tiejun Chen wrote:
> We don't do the real store operation for kprobing 'stwu Rx,(y)R1'
> since this may corrupt the exception frame, now we will do this
> operation safely in exception return code after migrate current
> exception frame below the kprobed function stack.
> 
> So we only update gpr[1] here and trigger a thread flag to mask
> this.
> 
> Note we should make sure if we trigger kernel stack over flow.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
>  arch/powerpc/lib/sstep.c |   37 +++++++++++++++++++++++++++++++++++--
>  1 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 9a52349..a4ce463 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -566,7 +566,7 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
>  	unsigned long int ea;
>  	unsigned int cr, mb, me, sh;
>  	int err;
> -	unsigned long old_ra;
> +	unsigned long old_ra, val3, r1;
>  	long ival;
>  
>  	opcode = instr >> 26;
> @@ -1486,11 +1486,44 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
>  		goto ldst_done;
>  
>  	case 36:	/* stw */
> -	case 37:	/* stwu */
>  		val = regs->gpr[rd];
>  		err = write_mem(val, dform_ea(instr, regs), 4, regs);
>  		goto ldst_done;
>  
> +	case 37:	/* stwu */
> +		__asm__ __volatile__("mr %0,1" : "=r" (r1) :);

I'll remove this line, please see below.

> +
> +		val = regs->gpr[rd];
> +		val3 = dform_ea(instr, regs);
> +		/*
> +		 * For PPC32 we always use stwu to change stack point with r1. So
> +		 * this emulated store may corrupt the exception frame, now we
> +		 * have to provide the exception frame trampoline, which is pushed
> +		 * below the kprobed function stack. So we only update gpr[1] but
> +		 * don't emulate the real store operation. We will do real store
> +		 * operation safely in exception return code by checking this flag.
> +		 */
> +		if ((ra == 1) && !(regs->msr & MSR_PR) && (val3 >= r1)) {
> +			/*
> +			 * Check if we will touch kernel sack overflow
> +			 */
> +			if (r1 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) {

OOPS. This line should be:
			
		if (val3 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) {

Tiejun

> +				printk(KERN_CRIT "Can't kprobe this since Kernel stack overflow.\n");
> +				err = -EINVAL;
> +				break;
> +			}
> +
> +			/*
> +			 * Check if we already set since that means we'll
> +			 * lose the previous value.
> +			 */
> +			WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE));
> +			set_thread_flag(TIF_EMULATE_STACK_STORE);
> +			err = 0;
> +		} else
> +			err = write_mem(val, val3, 4, regs);
> +		goto ldst_done;
> +
>  	case 38:	/* stb */
>  	case 39:	/* stbu */
>  		val = regs->gpr[rd];

Re: [v3 PATCH 3/3] ppc32/kprobe: don't emulate store when kprobe stwu r1

[tiejun.chen]

On 06/03/2012 02:59 PM, tiejun.chen wrote:
> On 06/03/2012 01:07 PM, Tiejun Chen wrote:
>> We don't do the real store operation for kprobing 'stwu Rx,(y)R1'
>> since this may corrupt the exception frame, now we will do this
>> operation safely in exception return code after migrate current
>> exception frame below the kprobed function stack.
>>
>> So we only update gpr[1] here and trigger a thread flag to mask
>> this.
>>
>> Note we should make sure if we trigger kernel stack over flow.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>>  arch/powerpc/lib/sstep.c |   37 +++++++++++++++++++++++++++++++++++--
>>  1 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
>> index 9a52349..a4ce463 100644
>> --- a/arch/powerpc/lib/sstep.c
>> +++ b/arch/powerpc/lib/sstep.c
>> @@ -566,7 +566,7 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
>>  	unsigned long int ea;
>>  	unsigned int cr, mb, me, sh;
>>  	int err;
>> -	unsigned long old_ra;
>> +	unsigned long old_ra, val3, r1;
>>  	long ival;
>>  
>>  	opcode = instr >> 26;
>> @@ -1486,11 +1486,44 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
>>  		goto ldst_done;
>>  
>>  	case 36:	/* stw */
>> -	case 37:	/* stwu */
>>  		val = regs->gpr[rd];
>>  		err = write_mem(val, dform_ea(instr, regs), 4, regs);
>>  		goto ldst_done;
>>  
>> +	case 37:	/* stwu */
>> +		__asm__ __volatile__("mr %0,1" : "=r" (r1) :);
> 
> I'll remove this line, please see below.
> 
>> +
>> +		val = regs->gpr[rd];
>> +		val3 = dform_ea(instr, regs);
>> +		/*
>> +		 * For PPC32 we always use stwu to change stack point with r1. So
>> +		 * this emulated store may corrupt the exception frame, now we
>> +		 * have to provide the exception frame trampoline, which is pushed
>> +		 * below the kprobed function stack. So we only update gpr[1] but
>> +		 * don't emulate the real store operation. We will do real store
>> +		 * operation safely in exception return code by checking this flag.
>> +		 */
>> +		if ((ra == 1) && !(regs->msr & MSR_PR) && (val3 >= r1)) {

And I also should change

(val3 >= r1) to (val3 >= (regs->r1 - STACK_INT_FRAME_SIZE)) since its worth
doing this only we'll really overwrite this exception stack.

Tiejun

>> +			/*
>> +			 * Check if we will touch kernel sack overflow
>> +			 */
>> +			if (r1 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) {
> 
> OOPS. This line should be:
> 			
> 		if (val3 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) {
> 
> Tiejun
> 
>> +				printk(KERN_CRIT "Can't kprobe this since Kernel stack overflow.\n");
>> +				err = -EINVAL;
>> +				break;
>> +			}
>> +
>> +			/*
>> +			 * Check if we already set since that means we'll
>> +			 * lose the previous value.
>> +			 */
>> +			WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE));
>> +			set_thread_flag(TIF_EMULATE_STACK_STORE);
>> +			err = 0;
>> +		} else
>> +			err = write_mem(val, val3, 4, regs);
>> +		goto ldst_done;
>> +
>>  	case 38:	/* stb */
>>  	case 39:	/* stbu */
>>  		val = regs->gpr[rd];
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 

[v3 PATCH 0/3] ppc32/kprobe: Fix a bug for kprobe stwu r1

[Tiejun Chen]

Changes from V2:

* populate those existed codes to reorganize codes
* add check if we'll trigger kernel stack over flow

Changes from V1:

* use memcpy simply to withdraw copy_exc_stack
* add !(regs->msr & MSR_PR)) and
	WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE));
  to make sure we're in goot path.
* move this migration process inside 'restore'
* clear TIF flag atomically 

Tiejun Chen (3):
      powerpc/kprobe: introduce a new thread flag
      ppc32/kprobe: complete kprobe and migrate exception frame
      ppc32/kprobe: don't emulate store when kprobe stwu r1

 arch/powerpc/include/asm/thread_info.h |    3 ++
 arch/powerpc/kernel/entry_32.S         |   43 ++++++++++++++++++++++++++-----
 arch/powerpc/lib/sstep.c               |   37 ++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 9 deletions(-)

Re: [v3 PATCH 0/3] ppc32/kprobe: Fix a bug for kprobe stwu r1

[tiejun.chen]

On 06/03/2012 01:07 PM, Tiejun Chen wrote:
> Changes from V2:
> 
> * populate those existed codes to reorganize codes
> * add check if we'll trigger kernel stack over flow

BTW, I always validate this on mpc8536ds(UP)/mpc8572ds(SMP) with/without
CONFIG_PREEMPT.

Tiejun

> 
> Changes from V1:
> 
> * use memcpy simply to withdraw copy_exc_stack
> * add !(regs->msr & MSR_PR)) and
> 	WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE));
>   to make sure we're in goot path.
> * move this migration process inside 'restore'
> * clear TIF flag atomically 
> 
> Tiejun Chen (3):
>       powerpc/kprobe: introduce a new thread flag
>       ppc32/kprobe: complete kprobe and migrate exception frame
>       ppc32/kprobe: don't emulate store when kprobe stwu r1
> 
>  arch/powerpc/include/asm/thread_info.h |    3 ++
>  arch/powerpc/kernel/entry_32.S         |   43 ++++++++++++++++++++++++++-----
>  arch/powerpc/lib/sstep.c               |   37 ++++++++++++++++++++++++++-
>  3 files changed, 74 insertions(+), 9 deletions(-)
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
>