----- On Mar 27, 2018, at 3:00 PM, rostedt rostedt@xxxxxxxxxxx wrote: > On Tue, 27 Mar 2018 11:45:34 -0700 > Alexei Starovoitov <ast@xxxxxx> wrote: > >> >> + >> >> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name); >> >> + addr = kallsyms_lookup_name(buf); >> >> + if (!addr) >> >> + return -ENOENT; >> >> + >> >> + return tracepoint_probe_register(tp, (void *)addr, prog); >> > >> > You are putting in a hell of a lot of trust with kallsyms returning >> > properly. I can see this being very fragile. This is calling a function >> > based on the result of kallsyms. I'm sure the security folks would love >> > this. >> > >> > There's a few things to make this a bit more robust. One is to add a >> > table that points to all __bpf_trace_* functions, and verify that the >> > result from kallsyms is in that table. >> > >> > Honestly, I think this is too much of a short cut and a hack. I know >> > you want to keep it "simple" and save space, but you really should do >> > it the same way ftrace and perf do it. That is, create a section and >> > have all tracepoints create a structure that holds a pointer to the >> > tracepoint and to the bpf probe function. Then you don't even need the >> > kernel_tracepoint_find_by_name(), you just iterate over your table and >> > you get the tracepoint and the bpf function associated to it. >> > >> > Relying on kallsyms to return an address to execute is just way too >> > extreme and fragile for my liking. >> >> Wasting extra 8bytes * number_of_tracepoints just for lack of trust >> in kallsyms doesn't sound like good trade off to me. >> If kallsyms are inaccurate all sorts of things will break: >> kprobes, livepatch, etc. >> I'd rather suggest for ftrace to use kallsyms approach as well >> and reduce memory footprint. > > If Linus, Thomas, Peter, Ingo, and the security folks trust kallsyms to > return a valid function pointer from a name, then sure, we can try > going that way. This will crash on ARM Thumb2 kernels. Also, how is this expected to work on PowerPC ABIv1 without KALLSYMS_ALL ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
