Skip to content

Conversation

@kernel-patches-daemon-bpf-rc
Copy link

Pull request for series with
subject: Fix ftrace for livepatch + BPF fexit programs
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1016392

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 8ce93aa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1016392
version: 4

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 2cbb259
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1016392
version: 4

When livepatch is attached to the same function as bpf trampoline with
a fexit program, bpf trampoline code calls register_ftrace_direct()
twice. The first time will fail with -EAGAIN, and the second time it
will succeed. This requires register_ftrace_direct() to unregister
the address on the first attempt. Otherwise, the bpf trampoline cannot
attach. Here is an easy way to reproduce this issue:

  insmod samples/livepatch/livepatch-sample.ko
  bpftrace -e 'fexit:cmdline_proc_show {}'
  ERROR: Unable to attach probe: fexit:vmlinux:cmdline_proc_show...

Fix this by cleaning up the hash when register_ftrace_function_nolock hits
errors.

Also, move the code that resets ops->func and ops->trampoline to the error
path of register_ftrace_direct(); and add a helper function reset_direct()
in register_ftrace_direct() and unregister_ftrace_direct().

Fixes: d05cb47 ("ftrace: Fix modification of direct_function hash while in use")
Cc: [email protected] # v6.6+
Reported-by: Andrey Grodzovsky <[email protected]>
Closes: https://lore.kernel.org/live-patching/[email protected]/
Cc: Steven Rostedt (Google) <[email protected]>
Cc: Masami Hiramatsu (Google) <[email protected]>
Acked-and-tested-by: Andrey Grodzovsky <[email protected]>
Signed-off-by: Song Liu <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
the same kernel function. When needed, ftrace_hash_ipmodify_enable()
calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
share the same function as the IPMODIFY ftrace (livepatch).

ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
but not called in modify_ftrace_direct() path. As a result, the following
operations will break livepatch:

1. Load livepatch to a kernel function;
2. Attach fentry program to the kernel function;
3. Attach fexit program to the kernel function.

After 3, the kernel function being used will not be the livepatched
version, but the original version.

Fix this by adding __ftrace_hash_update_ipmodify() to
__modify_ftrace_direct() and adjust some logic around the call.

Signed-off-by: Song Liu <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Both livepatch and BPF trampoline use ftrace. Special attention is needed
when livepatch and fexit program touch the same function at the same
time, because livepatch updates a kernel function and the BPF trampoline
need to call into the right version of the kernel function.

Use samples/livepatch/livepatch-sample.ko for the test.

The test covers two cases:
  1) When a fentry program is loaded first. This exercises the
     modify_ftrace_direct code path.
  2) When a fentry program is loaded first. This exercises the
     register_ftrace_direct code path.

Signed-off-by: Song Liu <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants