From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.windriver.com (mail.windriver.com [147.11.1.11]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mail.windriver.com", Issuer "Intel External Basic Issuing CA 3A" (not verified)) by ozlabs.org (Postfix) with ESMTPS id D4F6DB6F9F for ; Sun, 3 Jun 2012 17:01:01 +1000 (EST) Message-ID: <4FCB0B47.5080500@windriver.com> Date: Sun, 3 Jun 2012 14:59:19 +0800 From: "tiejun.chen" MIME-Version: 1.0 To: Subject: Re: [v3 PATCH 3/3] ppc32/kprobe: don't emulate store when kprobe stwu r1 References: <1338700063-30670-1-git-send-email-tiejun.chen@windriver.com> <1338700063-30670-3-git-send-email-tiejun.chen@windriver.com> In-Reply-To: <1338700063-30670-3-git-send-email-tiejun.chen@windriver.com> Content-Type: text/plain; charset="UTF-8" Cc: linuxppc-dev@ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 > --- > 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];