On Mon, 15 Aug 2022 08:17:42 -0700 Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > Ask yourself: should static_call patching logic go through > ftrace infra ? No. Right? I agree that static_call (and jump_labels) are not part of the ftrace infrastructure (but ftrace was a strong motivator for those). > static_call has nothing to do with ftrace (function tracing). Besides the motivation, I agree. > Same thing here. bpf dispatching logic is nothing to do with > function tracing. But it used fentry, which is part of function tracing. Which is what I'm against. And why it broke ftrace. > In this case bpf_dispatcher_xdp_func is a placeholder written C. > If it was written in asm, fentry recording wouldn't have known about it. And I would not have had an issue with that approach (for ftrace that is). But that brings up other concerns (see below). > And that's more or less what Jiri patch is doing. > It's hiding a fake function from ftrace, since it's not a function > and ftrace infra shouldn't show it tracing logs. > In other words it's a _notrace_ function with nop5. On the ftrace side, I'm perfectly happy with Jiri's approach (the one I help extend). But dynamic code modification is something we need to take very seriously. It's very similar to writing your own locking primitives (which Linus always says "Don't do"). It's complex and easy to get wrong. The more dynamic code modifications we have, the less secure the kernel is. Here's the list of dynamic code modification infrastructures: ftrace kprobes jump_labels static_calls We now have the bpf dispatcher. The ftrace, kprobes, jump_labels and static_calls developers work together to make sure that we are all in line, not breaking anything, and try to consolidate when possible. We also review each others code. The issue I have is that BPF is largely doing it alone, and not communicating with the others. This gives me cause for concern on both a robustness and security point of view. -- Steve
