[lkml]   [2017]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On 12/27/17 8:16 PM, Steven Rostedt wrote:
> On Wed, 27 Dec 2017 19:45:42 -0800
> Alexei Starovoitov <> 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.

 \ /
  Last update: 2017-12-28 05:34    [W:0.097 / U:4.048 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site