[RFC][Patch 1/2][take3]kprobe: kprobe-booster against 2.6.15-rc5-mm3 for i386

[RFC][Patch 1/2][take3]kprobe: kprobe-booster against 2.6.15-rc5-mm3 for i386

[Masami Hiramatsu]

Hi,

Here is a patch to clean up kprobe's resume_execute() for i386 arch.
Before applying kprobe-booster, I'd like to cleanup codes.
It is useful for simplify booster patch.

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp

 kprobes.c |   26 ++++++++++----------------
 1 files changed, 10 insertions(+), 16 deletions(-)
diff -Narup a/arch/i386/kernel/kprobes.c b/arch/i386/kernel/kprobes.c
--- a/arch/i386/kernel/kprobes.c	2005-12-20 19:55:42.000000000 +0900
+++ b/arch/i386/kernel/kprobes.c	2005-12-20 19:57:56.000000000 +0900
@@ -337,10 +337,10 @@ static void __kprobes resume_execution(s
 		struct pt_regs *regs, struct kprobe_ctlblk *kcb)
 {
 	unsigned long *tos = (unsigned long *)&regs->esp;
-	unsigned long next_eip = 0;
 	unsigned long copy_eip = (unsigned long)&p->ainsn.insn;
 	unsigned long orig_eip = (unsigned long)p->addr;

+	regs->eflags &= ~TF_MASK;
 	switch (p->ainsn.insn[0]) {
 	case 0x9c:		/* pushfl */
 		*tos &= ~(TF_MASK | IF_MASK);
@@ -350,9 +350,9 @@ static void __kprobes resume_execution(s
 	case 0xcb:
 	case 0xc2:
 	case 0xca:
-		regs->eflags &= ~TF_MASK;
-		/* eip is already adjusted, no more changes required*/
-		return;
+	case 0xea:		/* jmp absolute -- eip is correct */
+		/* eip is already adjusted, no more changes required */
+		goto no_change;
 	case 0xe8:		/* call relative - Fix return addr */
 		*tos = orig_eip + (*tos - copy_eip);
 		break;
@@ -360,27 +360,21 @@ static void __kprobes resume_execution(s
 		if ((p->ainsn.insn[1] & 0x30) == 0x10) {
 			/* call absolute, indirect */
 			/* Fix return addr; eip is correct. */
-			next_eip = regs->eip;
 			*tos = orig_eip + (*tos - copy_eip);
+			goto no_change;
 		} else if (((p->ainsn.insn[1] & 0x31) == 0x20) ||	/* jmp near, absolute indirect */
 			   ((p->ainsn.insn[1] & 0x31) == 0x21)) {	/* jmp far, absolute indirect */
 			/* eip is correct. */
-			next_eip = regs->eip;
+			goto no_change;
 		}
-		break;
-	case 0xea:		/* jmp absolute -- eip is correct */
-		next_eip = regs->eip;
-		break;
 	default:
 		break;
 	}

-	regs->eflags &= ~TF_MASK;
-	if (next_eip) {
-		regs->eip = next_eip;
-	} else {
-		regs->eip = orig_eip + (regs->eip - copy_eip);
-	}
+	regs->eip = orig_eip + (regs->eip - copy_eip);
+
+       no_change:
+	return ;
 }

 /*

Re: [RFC][Patch 1/2][take3]kprobe: kprobe-booster against 2.6.15-rc5-mm3 for i386

[Ananth N Mavinakayanahalli]

On Thu, Dec 22, 2005 at 10:23:56PM +0900, Masami Hiramatsu wrote:
> Hi,
> 
> Here is a patch to clean up kprobe's resume_execute() for i386 arch.
> Before applying kprobe-booster, I'd like to cleanup codes.
> It is useful for simplify booster patch.

Hi Masami,

This patch looks fine except for some coding-style issues (^^^ below).

Ananth

> 
> -- 
> Masami HIRAMATSU
> 2nd Research Dept.
> Hitachi, Ltd., Systems Development Laboratory
> E-mail: hiramatu@sdl.hitachi.co.jp
> 
>  kprobes.c |   26 ++++++++++----------------
>  1 files changed, 10 insertions(+), 16 deletions(-)
> diff -Narup a/arch/i386/kernel/kprobes.c b/arch/i386/kernel/kprobes.c
> --- a/arch/i386/kernel/kprobes.c	2005-12-20 19:55:42.000000000 +0900
> +++ b/arch/i386/kernel/kprobes.c	2005-12-20 19:57:56.000000000 +0900
> @@ -337,10 +337,10 @@ static void __kprobes resume_execution(s
>  		struct pt_regs *regs, struct kprobe_ctlblk *kcb)
>  {
>  	unsigned long *tos = (unsigned long *)&regs->esp;
> -	unsigned long next_eip = 0;
>  	unsigned long copy_eip = (unsigned long)&p->ainsn.insn;
>  	unsigned long orig_eip = (unsigned long)p->addr;
> 
> +	regs->eflags &= ~TF_MASK;
>  	switch (p->ainsn.insn[0]) {
>  	case 0x9c:		/* pushfl */
>  		*tos &= ~(TF_MASK | IF_MASK);
> @@ -350,9 +350,9 @@ static void __kprobes resume_execution(s
>  	case 0xcb:
>  	case 0xc2:
>  	case 0xca:
> -		regs->eflags &= ~TF_MASK;
> -		/* eip is already adjusted, no more changes required*/
> -		return;
> +	case 0xea:		/* jmp absolute -- eip is correct */
> +		/* eip is already adjusted, no more changes required */
> +		goto no_change;
>  	case 0xe8:		/* call relative - Fix return addr */
>  		*tos = orig_eip + (*tos - copy_eip);
>  		break;
> @@ -360,27 +360,21 @@ static void __kprobes resume_execution(s
>  		if ((p->ainsn.insn[1] & 0x30) == 0x10) {
>  			/* call absolute, indirect */
>  			/* Fix return addr; eip is correct. */
> -			next_eip = regs->eip;
>  			*tos = orig_eip + (*tos - copy_eip);
> +			goto no_change;
>  		} else if (((p->ainsn.insn[1] & 0x31) == 0x20) ||	/* jmp near, absolute indirect */
>  			   ((p->ainsn.insn[1] & 0x31) == 0x21)) {	/* jmp far, absolute indirect */
>  			/* eip is correct. */
> -			next_eip = regs->eip;
> +			goto no_change;
>  		}
> -		break;
> -	case 0xea:		/* jmp absolute -- eip is correct */
> -		next_eip = regs->eip;
> -		break;
>  	default:
>  		break;
>  	}
> 
> -	regs->eflags &= ~TF_MASK;
> -	if (next_eip) {
> -		regs->eip = next_eip;
> -	} else {
> -		regs->eip = orig_eip + (regs->eip - copy_eip);
> -	}
> +	regs->eip = orig_eip + (regs->eip - copy_eip);
> +
> +       no_change:
   ^^^^ no spaces for goto labels.
   
> +	return ;
	     ^^^ spurious whitespace
	     
>  }
> 
>  /*
> 
> 

Re: [RFC][Patch 1/2][take3]kprobe: kprobe-booster against 2.6.15-rc5-mm3 for i386

[Masami Hiramatsu]

Hi, Ananth

Ananth N Mavinakayanahalli wrote:
> On Thu, Dec 22, 2005 at 10:23:56PM +0900, Masami Hiramatsu wrote:
> 
>>Hi,
>>
>>Here is a patch to clean up kprobe's resume_execute() for i386 arch.
>>Before applying kprobe-booster, I'd like to cleanup codes.
>>It is useful for simplify booster patch.
> 
> Hi Masami,
> 
> This patch looks fine except for some coding-style issues (^^^ below).

Thank you for review. I will fix those coding style issues.

>>+
>>+       no_change:
>    ^^^^ no spaces for goto labels.
>
>
>>+	return ;
> 	     ^^^ spurious whitespace

Then, here is OK?
---
no_change:
	return;
---


-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp

Re: [RFC][Patch 1/2][take3]kprobe: kprobe-booster against 2.6.15-rc5-mm3 for i386

[Masami Hiramatsu]

Hi,

Here is a cleaned patch to clean resume_execution().
- fix indent of a label.
- remove a whitespace.

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp


 kprobes.c |   26 ++++++++++----------------
 1 files changed, 10 insertions(+), 16 deletions(-)
diff -Narup a/arch/i386/kernel/kprobes.c b/arch/i386/kernel/kprobes.c
--- a/arch/i386/kernel/kprobes.c	2006-01-24 19:07:26.000000000 +0900
+++ b/arch/i386/kernel/kprobes.c	2006-01-24 22:43:17.000000000 +0900
@@ -350,10 +350,10 @@ static void __kprobes resume_execution(s
 		struct pt_regs *regs, struct kprobe_ctlblk *kcb)
 {
 	unsigned long *tos = (unsigned long *)&regs->esp;
-	unsigned long next_eip = 0;
 	unsigned long copy_eip = (unsigned long)&p->ainsn.insn;
 	unsigned long orig_eip = (unsigned long)p->addr;

+	regs->eflags &= ~TF_MASK;
 	switch (p->ainsn.insn[0]) {
 	case 0x9c:		/* pushfl */
 		*tos &= ~(TF_MASK | IF_MASK);
@@ -363,9 +363,9 @@ static void __kprobes resume_execution(s
 	case 0xcb:
 	case 0xc2:
 	case 0xca:
-		regs->eflags &= ~TF_MASK;
-		/* eip is already adjusted, no more changes required*/
-		return;
+	case 0xea:		/* jmp absolute -- eip is correct */
+		/* eip is already adjusted, no more changes required */
+		goto no_change;
 	case 0xe8:		/* call relative - Fix return addr */
 		*tos = orig_eip + (*tos - copy_eip);
 		break;
@@ -373,27 +373,21 @@ static void __kprobes resume_execution(s
 		if ((p->ainsn.insn[1] & 0x30) == 0x10) {
 			/* call absolute, indirect */
 			/* Fix return addr; eip is correct. */
-			next_eip = regs->eip;
 			*tos = orig_eip + (*tos - copy_eip);
+			goto no_change;
 		} else if (((p->ainsn.insn[1] & 0x31) == 0x20) ||	/* jmp near, absolute indirect */
 			   ((p->ainsn.insn[1] & 0x31) == 0x21)) {	/* jmp far, absolute indirect */
 			/* eip is correct. */
-			next_eip = regs->eip;
+			goto no_change;
 		}
-		break;
-	case 0xea:		/* jmp absolute -- eip is correct */
-		next_eip = regs->eip;
-		break;
 	default:
 		break;
 	}

-	regs->eflags &= ~TF_MASK;
-	if (next_eip) {
-		regs->eip = next_eip;
-	} else {
-		regs->eip = orig_eip + (regs->eip - copy_eip);
-	}
+	regs->eip = orig_eip + (regs->eip - copy_eip);
+
+no_change:
+	return;
 }

 /*

Re: [RFC][Patch 1/2][take3]kprobe: kprobe-booster against 2.6.15-rc5-mm3 for i386

[Ananth N Mavinakayanahalli]

On Tue, Jan 24, 2006 at 10:23:02PM +0900, Masami Hiramatsu wrote:
> Hi, Ananth
> 
> Ananth N Mavinakayanahalli wrote:
> > On Thu, Dec 22, 2005 at 10:23:56PM +0900, Masami Hiramatsu wrote:
> > 
> >>Hi,
> >>
> >>Here is a patch to clean up kprobe's resume_execute() for i386 arch.
> >>Before applying kprobe-booster, I'd like to cleanup codes.
> >>It is useful for simplify booster patch.
> > 
> > Hi Masami,
> > 
> > This patch looks fine except for some coding-style issues (^^^ below).
> 
> Thank you for review. I will fix those coding style issues.
> 
> >>+
> >>+       no_change:
> >    ^^^^ no spaces for goto labels.
> >
> >
> >>+	return ;
> > 	     ^^^ spurious whitespace
> 
> Then, here is OK?
> ---
> no_change:
> 	return;
> ---

Yes

Ananth