On Sat, 3 Nov 2018 09:17:54 -0400
Steven Rostedt <rost...@goodmis.org> wrote:
> > > Then I tested with this:
> > >
> > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > > index 3ef15a6683c0..4ddafddf1343 100644
> > > --- a/kernel/trace/trace_probe.c
> > > +++ b/kernel/trace/trace_probe.c
> > > @@ -516,6 +516,7 @@ void traceprobe_free_probe_arg(struct probe_arg
> > > *arg) kfree(code->data);
> > > code++;
> > > }
> > > + printk("free arg->code %s\n", arg->code);
> > > kfree(arg->code);
> > > kfree(arg->name);
> > > kfree(arg->comm);
> > > @@ -535,11 +536,15 @@ int traceprobe_update_arg(struct probe_arg *arg)
> > > if (code[1].op != FETCH_OP_IMM)
> > > return -EINVAL;
> > >
> > > + tmp = strpbrk(code->data, "+-");
> > > + printk("first tmp tmp=%s\n", tmp);
> > > tmp = strpbrk("+-", code->data);
> > > + printk("second tmp=%s data=%s\n", tmp,
> > > code->data); if (tmp)
> > > c = *tmp;
> > > ret =
> > > traceprobe_split_symbol_offset(code->data, &offset);
> > > + printk("third data=%s\n", code->data);
> > > if (ret)
> > > return ret;
> > >
> > > @@ -547,6 +552,7 @@ int traceprobe_update_arg(struct probe_arg *arg)
> > > (unsigned
> > > long)kallsyms_lookup_name(code->data); if (tmp)
> > > *tmp = c;
> > > + printk("forth data=%s\n", code->data);
> > > if (!code[1].immediate)
> > > return -ENOENT;
> > > code[1].immediate += offset;
> > >
> > > And I don't see where that code->data is used elsewhere. That is, why
> > > even bother saving the character?
> >
> > Would you mean parsing the symbol+offs every time is useless?
> > It needs to solve the symbol address always because traceprobe_update_arg
> > is called when new symbols added on the kernel (by loading modules).
>
> OK, so it is called multiple times? That is when a module is loaded?
> Because I couldn't get this called multiple times.
Sorry I mislead you.
See trace_kprobe_module_callback(), which calls __register_trace_kprobe()
if the probepoint is on the module. And __register_trace_kprobe() calls
traceprobe_update_arg().
So, if you call it multiple time, it should be with the probe point is
on the module too.
e.g.
echo "p newmod:function @var_symbol+offset" > kprobe_events
can be called multiple times if we load/unload "newmod" module.
> I'll try that out and if that's the case, then yeah, this needs to be
> fixed (otherwise, I was thinking we could just remove the strpbrk()
> altogether).
>
>
> >
> > Hmm, maybe I can introduce a struct like
> >
> > struct symbol_offset {
> > long offset;
> > char symbol[];
> > };
> >
> > and use it instead of parsing the symbol+offset always.
>
> The parsing should be fine. The issue I had was that I couldn't trigger
> it to happen more than once. That's why this passed testing. We didn't
> do something that required it to be called more than once and that is
> here the bug would trigger.
Hmm, I hit it by kprobe_args_syntax.tc, so hit once is enough to kick
this bug. Maybe some config option makes "+-" readonly, which previously
didn't enabled.
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
