[PATCH] ftrace: Fix off-by-one fentry site disable in ftrace_free_mem()

[PATCH] ftrace: Fix off-by-one fentry site disable in ftrace_free_mem()

[Josh Poimboeuf]

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.

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 */
 
 	mutex_lock(&ftrace_lock);
 
-- 
2.54.0

Re: [PATCH] ftrace: Fix off-by-one fentry site disable in ftrace_free_mem()

[Steven Rostedt]

On Sun,  2 Aug 2026 20:08:35 -0700
Josh Poimboeuf <jpoimboe@kernel.org> 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.


> 
> 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;

Re: [PATCH] ftrace: Fix off-by-one fentry site disable in ftrace_free_mem()

[Josh Poimboeuf]

On Wed, Aug 05, 2026 at 03:30:05PM -0400, Steven Rostedt wrote:
> On Sun,  2 Aug 2026 20:08:35 -0700
> Josh Poimboeuf <jpoimboe@kernel.org> 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.
> 
> 
> > 
> > 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?

Yeah, that would be better, let me go do that.

> 
> -- 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;

-- 
Josh