When removing a graph operation while other operations remain, ftrace_shutdown_subops() rebuilds the manager hashes. Memory allocations in rebuild_hashes() or ftrace_update_ops() can fail with -ENOMEM.
On a rebuild failure, the subop has been removed from the list but still has FTRACE_OPS_FL_ENABLED and FTRACE_OPS_FL_SUBOP set. On an update failure, it is added back to the list. Neither outcome matches unregister_ftrace_graph(), which has already removed the graph slot. For example, enable a function_graph tracer with a nonempty filter and a return fprobe. Fail an allocation in rebuild_hashes() while disabling the fprobe, disable the tracer, then re-enable the fprobe. The stale SUBOP state routes its filter update through the now-empty manager list. rebuild_hashes() leaves its output pointers uninitialized, and ops_equal() can crash when dereferencing them. Complete non-final subop removal on both error paths by clearing its enabled/subop flags and manager pointer without adding it back to the list. Initialize the rebuild outputs to EMPTY_HASH as well. A failed manager hash update may leave functions enabled that only the removed operation traced. Keep per-subop filtering in that case, since direct graph dispatch skips those checks. Restore direct dispatch after a successful removal from two graph operations to one, or after a successful first registration. This depends on the reader synchronization in the preceding patch. Clearing the enabled flag after a failure allows the caller to release the subop filter, while an existing graph callback may still hold it. Backport both patches together. Oops: general protection fault, probably for non-canonical address 0xdffffc0000000802: 0000 [#1] SMP KASAN NOPTI KASAN: probably user-memory-access in range [0x0000000000004010-0x0000000000004017] RIP: ops_equal (kernel/trace/trace.h:1011 kernel/trace/ftrace.c:3416) Call Trace: ftrace_update_ops (kernel/trace/ftrace.c:3464) ftrace_hash_move_and_update_subops.isra.0 (kernel/trace/ftrace.c:3795) ftrace_hash_move_and_update_ops (kernel/trace/ftrace.c:5095) ftrace_set_hash (kernel/trace/ftrace.c:6006) ftrace_set_filter_ips (kernel/trace/ftrace.c:6020 kernel/trace/ftrace.c:6769) register_fprobe_ips (kernel/trace/fprobe.c:290 kernel/trace/fprobe.c:1020) register_fprobe (kernel/trace/fprobe.c:979) enable_trace_fprobe (kernel/trace/trace_fprobe.c:838 kernel/trace/trace_fprobe.c:1480) fprobe_register (kernel/trace/trace_fprobe.c:1555) __ftrace_event_enable_disable (kernel/trace/trace_events.c:913) event_enable_write (kernel/trace/trace_events.c:943 kernel/trace/trace_events.c:1995) vfs_write (fs/read_write.c:685) ksys_write (fs/read_write.c:739) do_syscall_64 (arch/x86/entry/syscall_64.c:61 arch/x86/entry/syscall_64.c:84) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121) Kernel panic - not syncing: Fatal exception Cc: [email protected] Fixes: 0ae6b8ce200d ("ftrace: Fix accounting of subop hashes") Reported-by: <[email protected]> Assisted-by: LLM Signed-off-by: Weiming Shi <[email protected]> --- kernel/trace/fgraph.c | 10 +++++++--- kernel/trace/ftrace.c | 22 ++++++++++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c index 14733f519..48521bc33 100644 --- a/kernel/trace/fgraph.c +++ b/kernel/trace/fgraph.c @@ -1406,8 +1406,11 @@ int register_ftrace_graph(struct fgraph_ops *gops) gops->ops.flags |= FTRACE_OPS_FL_GRAPH; ret = ftrace_startup_subops(&graph_ops, &gops->ops, command); - if (!ret) + if (!ret) { fgraph_array[i] = gops; + if (ftrace_graph_active == 1) + ftrace_graph_enable_direct(true, gops); + } error: if (ret) { @@ -1435,6 +1438,7 @@ int register_ftrace_graph(struct fgraph_ops *gops) bool unregister_ftrace_graph_start(struct fgraph_ops *gops) { int command = 0; + int ret = -EINVAL; bool removed = false; if (WARN_ONCE(!(gops->ops.flags & FTRACE_OPS_FL_GRAPH), @@ -1461,9 +1465,9 @@ bool unregister_ftrace_graph_start(struct fgraph_ops *gops) if (!ftrace_graph_active) command = FTRACE_STOP_FUNC_RET; - ftrace_shutdown_subops(&graph_ops, &gops->ops, command); + ret = ftrace_shutdown_subops(&graph_ops, &gops->ops, command); - if (ftrace_graph_active == 1) + if (ftrace_graph_active == 1 && !ret) ftrace_graph_enable_direct(true, NULL); else if (!ftrace_graph_active) ftrace_graph_disable_direct(false); diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index c7d74b33d..7c712d99f 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -3672,6 +3672,8 @@ static int rebuild_hashes(struct ftrace_hash **filter_hash, struct ftrace_hash * bool first = true; int ret; + *filter_hash = EMPTY_HASH; + *notrace_hash = EMPTY_HASH; temp_hash.filter_hash = EMPTY_HASH; temp_hash.notrace_hash = EMPTY_HASH; @@ -3713,7 +3715,13 @@ static int rebuild_hashes(struct ftrace_hash **filter_hash, struct ftrace_hash * * will not affect functions that are being traced by other subops that * still exist in @ops. * - * If the last subops is removed from @ops, then @ops is shutdown normally. + * If @subops is the last subop, @ops is shut down normally. Otherwise, + * @subops is detached even if the manager hash update fails. The manager + * hashes may be partially updated, but still cover all functions required by + * the remaining subops. + * + * Return: 0 on success, or a negative error code on shutdown or hash update + * failure. */ int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command) { @@ -3753,15 +3761,13 @@ int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, in /* Rebuild the hashes without subops */ ret = rebuild_hashes(&filter_hash, ¬race_hash, ops); if (ret < 0) - return ret; + goto out; ret = ftrace_update_ops(ops, filter_hash, notrace_hash); - if (ret < 0) { - list_add(&subops->list, &ops->subop_list); - } else { - subops->flags &= ~(FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_SUBOP); - subops->managed = NULL; - } + +out: + subops->flags &= ~(FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_SUBOP); + subops->managed = NULL; free_ftrace_hash(filter_hash); free_ftrace_hash(notrace_hash); return ret; -- 2.56.0
