[PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[Abhishek Sagar]

Hi Steven/Ingo,

These patches address the problem of kprobe registration interfering with
dynamic ftrace. A kprobe registered on an mcount call-site can modify that
address multiple times during its duration of probing. At the same time,
ftrace may simultaneously modify these locations. This could be fatal
especially if ftrace modifies the call-site during a kprobe registration
on it (between arch_prepare_kprobe and arch_arm_kprobe in __register_kprobe
to be more precise). So as a "fix", I've disabled any updates on the mcount
call-site while it's being kprobe'd.

--
Regards,
Abhishek Sagar

Re: [PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[Ingo Molnar]

* Abhishek Sagar <sagar.abhishek@gmail.com> wrote:

> Hi Steven/Ingo,
> 
> These patches address the problem of kprobe registration interfering 
> with dynamic ftrace. A kprobe registered on an mcount call-site can 
> modify that address multiple times during its duration of probing. At 
> the same time, ftrace may simultaneously modify these locations. This 
> could be fatal especially if ftrace modifies the call-site during a 
> kprobe registration on it (between arch_prepare_kprobe and 
> arch_arm_kprobe in __register_kprobe to be more precise). So as a 
> "fix", I've disabled any updates on the mcount call-site while it's 
> being kprobe'd.

applied to tip/tracing/ftrace, thanks Abhishek.

note that i had to do a number of fixups to themerge, and there was one 
chunk which did not merge cleanly and i left it out for now:

@@ -435,8 +502,12 @@ static void ftrace_replace_code(int enable)
                                continue;

                        /* ignore updates to this record's mcount site */
-                       if (get_kprobe((void *)rec->ip))
+                       if (get_kprobe((void *)rec->ip)) {
+                               freeze_record(rec);
                                continue;
+                       } else {
+                               unfreeze_record(rec);
+                       }

please send a delta patch against tip/tracing/ftrace to get that kprobes 
fix. You can pick up -tip via:

 http://people.redhat.com/mingo/tip.git/README

	Ingo

Re: [PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[Abhishek Sagar]

Hi Ingo,

On Tue, Jun 24, 2008 at 1:45 AM, Ingo Molnar <mingo@elte.hu> wrote:
> note that i had to do a number of fixups to themerge, and there was one
> chunk which did not merge cleanly and i left it out for now:
>
> @@ -435,8 +502,12 @@ static void ftrace_replace_code(int enable)
>                                continue;
>
>                        /* ignore updates to this record's mcount site */
> -                       if (get_kprobe((void *)rec->ip))
> +                       if (get_kprobe((void *)rec->ip)) {
> +                               freeze_record(rec);
>                                continue;
> +                       } else {
> +                               unfreeze_record(rec);
> +                       }

I wonder why it conflicted during the merge. I notice that patch 4/4
has been applied before
3/4. Can you try reversing the order and see if it works? Else I'll redo them.

--
Thanks,
Abhishek Sagar

Re: [PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[Ingo Molnar]

* Abhishek Sagar <sagar.abhishek@gmail.com> wrote:

> Hi Ingo,
> 
> On Tue, Jun 24, 2008 at 1:45 AM, Ingo Molnar <mingo@elte.hu> wrote:
> > note that i had to do a number of fixups to themerge, and there was one
> > chunk which did not merge cleanly and i left it out for now:
> >
> > @@ -435,8 +502,12 @@ static void ftrace_replace_code(int enable)
> >                                continue;
> >
> >                        /* ignore updates to this record's mcount site */
> > -                       if (get_kprobe((void *)rec->ip))
> > +                       if (get_kprobe((void *)rec->ip)) {
> > +                               freeze_record(rec);
> >                                continue;
> > +                       } else {
> > +                               unfreeze_record(rec);
> > +                       }
> 
> I wonder why it conflicted during the merge. I notice that patch 4/4 
> has been applied before 3/4. Can you try reversing the order and see 
> if it works? Else I'll redo them.

best would be a delta patch against tip/master to fix up whatever is 
still missing.

	Ingo

Re: [PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[Abhishek Sagar]

Ingo Molnar wrote:
> best would be a delta patch against tip/master to fix up whatever is 
> still missing.

Here is the merge fixup for tip/master (and tip/tracing/ftrace).

Signed-off-by: Abhishek Sagar <sagar.abhishek@gmail.com>
---

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 85e8413..0f271c4 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -502,8 +502,12 @@ static void ftrace_replace_code(int enable)
 				continue;
 
 			/* ignore updates to this record's mcount site */
-			if (get_kprobe((void *)rec->ip))
+			if (get_kprobe((void *)rec->ip)) {
+				freeze_record(rec);
 				continue;
+			} else {
+				unfreeze_record(rec);
+			}
 
 			failed = __ftrace_replace_code(rec, old, new, enable);
 			if (failed && (rec->flags & FTRACE_FL_CONVERTED)) {
@@ -740,7 +744,10 @@ static int __ftrace_update_code(void *ignore)
 				ftrace_del_hash(p);
 				INIT_HLIST_NODE(&p->node);
 				hlist_add_head(&p->node, &temp_list);
+				freeze_record(p);
 				continue;
+			} else {
+				unfreeze_record(p);
 			}
 
 			/* convert record (i.e, patch mcount-call with NOP) */

Re: [PATCH 0/4] ftrace: prevent ftrace modifications while being kprobe'd

[Ingo Molnar]

* Abhishek Sagar <sagar.abhishek@gmail.com> wrote:

> Ingo Molnar wrote:
> > best would be a delta patch against tip/master to fix up whatever is 
> > still missing.
> 
> Here is the merge fixup for tip/master (and tip/tracing/ftrace).
> 
> Signed-off-by: Abhishek Sagar <sagar.abhishek@gmail.com>

applied to tip/tracing/ftrace - thanks Abhishek!

	Ingo