Re: [PATCH] ftrace: Fix off-by-one fentry site disable in ftrace_free_mem()
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2026-08-05 19:29:18
Also in:
lkml
Subsystem:
function hooks (ftrace), the rest, tracing · Maintainers:
Steven Rostedt, Masami Hiramatsu, Linus Torvalds
On Sun, 2 Aug 2026 20:08:35 -0700 Josh Poimboeuf [off-list ref] wrote:
When a module's init text is freed, do_init_module() calls ftrace_free_mem() with a half-open [start, end) range. However the ftrace_cmp_recs() comparator treats the upper bound as inclusive, as all its other users do, passing 'ip + size - 1'. So ftrace_free_mem() can delete a record sitting exactly at 'end', which is outside the freed range. For a kernel without CFI or IBT, the first record of a function is at the function start, which for the first function in a module is also the base of its text allocation. As the module allocator packs its regions, that address is often the 'end' passed by a neighboring module's do_init_module(), causing the first function's ftrace location to get disabled, preventing an attempt to livepatch it: livepatch: failed to find location for function 'pcspkr_probe' Convert the exclusive end to the inclusive 'end - 1' the comparator expects, and return early for an empty range to avoid the subtraction from underflowing when the init text size is zero.
Nice catch.
quoted hunk ↗ jump to hunk
Fixes: 42c269c88dc1 ("ftrace: Allow for function tracing to record init functions on boot up") Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- kernel/trace/ftrace.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index f93e34dd2328..7d8b736f0d86 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c@@ -8293,8 +8293,11 @@ void ftrace_free_mem(struct module *mod, void *start_ptr, void *end_ptr) struct ftrace_init_func *func, *func_next; LIST_HEAD(clear_hash); + if (start >= end) + return; + key.ip = start; - key.flags = end; /* overload flags, as it is unsigned long */ + key.flags = end - 1; /* overload flags, as it is unsigned long */
I'd like to keep this consistent with lookup_rec().
mutex_lock(&ftrace_lock);
Can you do this instead? -- Steve
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6c47a94f5924..dbb0fc2928d8 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c@@ -8296,7 +8296,8 @@ static void add_to_clear_hash_list(struct list_head *clear_list, void ftrace_free_mem(struct module *mod, void *start_ptr, void *end_ptr) { unsigned long start = (unsigned long)(start_ptr); - unsigned long end = (unsigned long)(end_ptr); + /* end is inclusive and end_ptr is exclusive */ + unsigned long end = (unsigned long)(end_ptr) - 1; struct ftrace_page **last_pg = &ftrace_pages_start; struct ftrace_page *tmp_page = NULL; struct ftrace_page *pg;