Messages in this thread | | | Subject | Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry | From | Alexei Starovoitov <> | Date | Wed, 27 Dec 2017 20:32:07 -0800 |
| |
On 12/27/17 8:16 PM, Steven Rostedt wrote: > On Wed, 27 Dec 2017 19:45:42 -0800 > Alexei Starovoitov <ast@fb.com> wrote: > >> I don't think that's the case. My reading of current >> trace_kprobe_ftrace() -> arch_check_ftrace_location() >> is that it will not be true for old mcount case. > > In the old mcount case, you can't use ftrace to return without calling > the function. That is, no modification of the return ip, unless you > created a trampoline that could handle arbitrary stack frames, and > remove them from the stack before returning back to the function.
correct. I was saying that trace_kprobe_ftrace() won't let us do bpf_override_return with old mcount.
>> >> As far as the rest of your arguments it very much puzzles me that >> you claim that this patch suppose to work based on historical >> reasoning whereas you did NOT test it. > > I believe that Masami is saying that the modification of the IP from > kprobes has been very well tested. But I'm guessing that you still want > a test case for using kprobes in this particular instance. It's not the > implementation of modifying the IP that you are worried about, but the > implementation of BPF using it in this case. Right?
exactly. No doubt that old code works. But it doesn't mean that bpf_override_return() will continue to work in kprobes that are not ftrace based. I suspect Josef's existing test case will cover this situation. Probably only special .config is needed to disable ftrace, so "kprobe on entry but not ftrace" check will kick in. But I didn't get an impression that this situation was tested. Instead I see only logical reasoning that it's _supposed_ to work. That's not enough.
| |