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

["tiejun.chen" <tiejun.chen@windriver.com>]

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
> 
>