Re: [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support

[David Long]

On 05/13/15 11:41, William Cohen wrote:
> On 05/13/2015 05:22 AM, Masami Hiramatsu wrote:
> > On 2015/05/12 21:48, William Cohen wrote:
>
> > > Hi Dave,
> > >
> > > In some of the previous diagnostic output it looked like things would go wrong
> > > in the entry.S when the D bit was cleared and the debug interrupts were
> > > unmasksed.  I wonder if some of the issue might be due to the starting the
> > > kprobe for the trampoline, but leaving things in an odd state when another
> > > set of krpobe/kretporbes are hit when the trampoline is running.
> >
> > Hmm, does this mean we have a trouble if a user kprobe handler calls the
> > function which is probed by other kprobe? Or, is this just a problem
> > only for kretprobes?
>
> Hi Masami,
>
> I wrote an example based off of sample/kprobes/kprobes_sample.c to force the reentry issue for kprobes (the attached kprobe_rentry_example.c). That seemed to run fine.  I think the reason that the trampoline handler got into trouble is because of the reset_current_kprobe() before the possible call to kfree, but I haven't verified it. It seems like that should be at the end of trampoline handler just before the return.  Other architectures have similar trampoline handlers, so I am surprised that the other architectures haven't encountered this issue with kretprobes.  Maybe this is due to specific of arm64 exception handling.
>
> # modprobe kprobe_reentry_example
> [  909.617295] Planted kprobe at fffffe00000b7b34
> [  909.623873] Planted kprobe at fffffe000032d34c
> # rmmod kprobe_reentry_example
> [ 1482.647504] kprobe at fffffe00000b7b34 unregistered
> [ 1482.687506] kprobe at fffffe000032d34c unregistered
> [ 1482.692361] y = 42
> [ 1482.694361] z = 0
> # grep \ int_sqrt$ /proc/kallsyms
> fffffe000032d34c T int_sqrt
> # grep \ do_fork$ /proc/kallsyms
> fffffe00000b7b34 T do_fork

I actually have been doing exactly the same thing.  I was not able to 
provoke a failure in either the pre or post handler when having them 
call to another kprobe'd (dummy) function.  However it does look to me 
like it's reentering the exception handler firstly at the second 
software breakpoint and then (shortly after returning from that) with a 
single-step.   That's what you would expect but I *thought* neither of 
those cases was allowed.  It does not call either the pre or post 
handler in the dummy function.  I would expect this to be an issue for a 
kretprobe pre handler too, although I've yet to test this.

> > >   As Dave
> > > mentioned the proposed trampoline patch avoids using a kprobe in the
> > > trampoline and directly calls the trampoline handler.  Attached is the
> > > current version of the patch which was able to run the systemtap testsuite.
> > >   Systemtap does exercise the kprobe/kretprobe infrastructure, but it would
> > > be good to have additional raw kprobe tests to check that kprobe reentry
> > > works as expected.
> >
> > Actually, Will's patch looks like the same thing what I did on x86,
> > as the kretprobe-booster. So I'm OK for that. But if the above problem
> > is not solved, we need to fix that, since kprobes can be used from
> > different sources.
>
> The patch should look similar to the x86 code. The x86 code was used as a model.
>
> -Will

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/