[PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[qingwei.hu]

From: Qingwei Hu <qingwei.hu@bytedance.com>

There is a possible configuration dependency:

  KPROBES_ON_FTRACE [=n]
       ^----- KPROBES [=y]
         |--- HAVE_KPROBES_ON_FTRACE [=n]
         |--- DYNAMIC_FTRACE_WITH_REGS [=n]
                ^----- FTRACE [=y]
                  |--- DYNAMIC_FTRACE [=y]
                  |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]

With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
return the same address as the probe target.

However, when KPROBES_ON_FTRACE=n, the current implementation
returns -EINVAL after calling check_ftrace_location(), causing
the validation to fail.

Adjust the logic so that ftrace-based checks are performed only
when CONFIG_KPROBES_ON_FTRACE is enabled, ensuring correct
kprobe behavior even when KPROBES_ON_FTRACE=n.

Signed-off-by: Qingwei Hu <qingwei.hu@bytedance.com>
---
 kernel/kprobes.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ab8f9fc1f0d1..f4aa4ba1ca9c 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1512,19 +1512,15 @@ static inline int warn_kprobe_rereg(struct kprobe *p)
 	return 0;
 }
 
-static int check_ftrace_location(struct kprobe *p)
+#ifdef CONFIG_KPROBES_ON_FTRACE
+static void check_ftrace_location(struct kprobe *p)
 {
 	unsigned long addr = (unsigned long)p->addr;
 
-	if (ftrace_location(addr) == addr) {
-#ifdef CONFIG_KPROBES_ON_FTRACE
+	if (ftrace_location(addr) == addr)
 		p->flags |= KPROBE_FLAG_FTRACE;
-#else
-		return -EINVAL;
-#endif
-	}
-	return 0;
 }
+#endif
 
 static bool is_cfi_preamble_symbol(unsigned long addr)
 {
@@ -1540,11 +1536,9 @@ static bool is_cfi_preamble_symbol(unsigned long addr)
 static int check_kprobe_address_safe(struct kprobe *p,
 				     struct module **probed_mod)
 {
-	int ret;
-
-	ret = check_ftrace_location(p);
-	if (ret)
-		return ret;
+#ifdef CONFIG_KPROBES_ON_FTRACE
+	check_ftrace_location(p);
+#endif
 
 	guard(jump_label_lock)();
 
-- 
2.39.5

Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[Steven Rostedt]

On Fri,  5 Dec 2025 17:29:33 +0800
"qingwei.hu" <qingwei.hu@bytedance.com> wrote:

> From: Qingwei Hu <qingwei.hu@bytedance.com>
> 
> There is a possible configuration dependency:
> 
>   KPROBES_ON_FTRACE [=n]
>        ^----- KPROBES [=y]
>          |--- HAVE_KPROBES_ON_FTRACE [=n]
>          |--- DYNAMIC_FTRACE_WITH_REGS [=n]
>                 ^----- FTRACE [=y]
>                   |--- DYNAMIC_FTRACE [=y]
>                   |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
> 
> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
> return the same address as the probe target.
> 
> However, when KPROBES_ON_FTRACE=n, the current implementation
> returns -EINVAL after calling check_ftrace_location(), causing
> the validation to fail.

This is a feature not a bug.

The reason is if you put a kprobe on a ftrace location, it can cause ftrace
to trigger a bug, as kprobes will modify the location and ftrace will see
something it doesn't expect and think the system is corrupted. We don't want
that either.

If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
controlled by ftrace, it had better fail!

NAK

-- Steve

Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[Masami Hiramatsu]

On Fri,  5 Dec 2025 17:29:33 +0800
"qingwei.hu" <qingwei.hu@bytedance.com> wrote:

> From: Qingwei Hu <qingwei.hu@bytedance.com>
> 
> There is a possible configuration dependency:
> 
>   KPROBES_ON_FTRACE [=n]
>        ^----- KPROBES [=y]
>          |--- HAVE_KPROBES_ON_FTRACE [=n]
>          |--- DYNAMIC_FTRACE_WITH_REGS [=n]
>                 ^----- FTRACE [=y]
>                   |--- DYNAMIC_FTRACE [=y]
>                   |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
> 
> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
> return the same address as the probe target.
> 
> However, when KPROBES_ON_FTRACE=n, the current implementation
> returns -EINVAL after calling check_ftrace_location(), causing
> the validation to fail.
> 
> Adjust the logic so that ftrace-based checks are performed only
> when CONFIG_KPROBES_ON_FTRACE is enabled, ensuring correct
> kprobe behavior even when KPROBES_ON_FTRACE=n.

It is a bit complicated but CONFIG_KPROBES_ON_FTRACE is a hidden
static option set by architecture. If it is not enabled, that
feature is not usable on the architecture. And as Steve mentioned
we can not put a software breakpoint code on where the ftrace will
modify. IOW, ftrace reserves the instruction address.
Thus, we will check whether it is reserved by ftrace, and if so,
it returns -EINVAL, or if architecture already implements the
kprobes on ftrace feature (it requires to implement a ftrace handler
which mimics kprobe), it uses that.

So I can not accept this.

Thank you,

> 
> Signed-off-by: Qingwei Hu <qingwei.hu@bytedance.com>
> ---
>  kernel/kprobes.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ab8f9fc1f0d1..f4aa4ba1ca9c 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1512,19 +1512,15 @@ static inline int warn_kprobe_rereg(struct kprobe *p)
>  	return 0;
>  }
>  
> -static int check_ftrace_location(struct kprobe *p)
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +static void check_ftrace_location(struct kprobe *p)
>  {
>  	unsigned long addr = (unsigned long)p->addr;
>  
> -	if (ftrace_location(addr) == addr) {
> -#ifdef CONFIG_KPROBES_ON_FTRACE
> +	if (ftrace_location(addr) == addr)
>  		p->flags |= KPROBE_FLAG_FTRACE;
> -#else
> -		return -EINVAL;
> -#endif
> -	}
> -	return 0;
>  }
> +#endif
>  
>  static bool is_cfi_preamble_symbol(unsigned long addr)
>  {
> @@ -1540,11 +1536,9 @@ static bool is_cfi_preamble_symbol(unsigned long addr)
>  static int check_kprobe_address_safe(struct kprobe *p,
>  				     struct module **probed_mod)
>  {
> -	int ret;
> -
> -	ret = check_ftrace_location(p);
> -	if (ret)
> -		return ret;
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +	check_ftrace_location(p);
> +#endif
>  
>  	guard(jump_label_lock)();
>  
> -- 
> 2.39.5
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[qingwei hu]

> 2025年12月5日 23:08，Steven Rostedt <rostedt@goodmis.org> 写道：
> 
> On Fri,  5 Dec 2025 17:29:33 +0800
> "qingwei.hu" <qingwei.hu@bytedance.com> wrote:
> 
>> From: Qingwei Hu <qingwei.hu@bytedance.com>
>> 
>> There is a possible configuration dependency:
>> 
>>  KPROBES_ON_FTRACE [=n]
>>       ^----- KPROBES [=y]
>>         |--- HAVE_KPROBES_ON_FTRACE [=n]
>>         |--- DYNAMIC_FTRACE_WITH_REGS [=n]
>>                ^----- FTRACE [=y]
>>                  |--- DYNAMIC_FTRACE [=y]
>>                  |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
>> 
>> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
>> return the same address as the probe target.
>> 
>> However, when KPROBES_ON_FTRACE=n, the current implementation
>> returns -EINVAL after calling check_ftrace_location(), causing
>> the validation to fail.
> 
> This is a feature not a bug.
> 
> The reason is if you put a kprobe on a ftrace location, it can cause ftrace
> to trigger a bug, as kprobes will modify the location and ftrace will see
> something it doesn't expect and think the system is corrupted. We don't want
> that either.
> 
> If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
> controlled by ftrace, it had better fail!
> 
> NAK
> 
> -- Steve

Thanks for your clear explanation. I will look into other approaches
that work with this configuration.

Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[Masami Hiramatsu]

On Mon, 8 Dec 2025 14:54:33 +0800
qingwei hu <huqingwei.kernel@gmail.com> wrote:

> 
> 
> > 2025年12月5日 23:08，Steven Rostedt <rostedt@goodmis.org> 写道：
> > 
> > On Fri,  5 Dec 2025 17:29:33 +0800
> > "qingwei.hu" <qingwei.hu@bytedance.com> wrote:
> > 
> >> From: Qingwei Hu <qingwei.hu@bytedance.com>
> >> 
> >> There is a possible configuration dependency:
> >> 
> >>  KPROBES_ON_FTRACE [=n]
> >>       ^----- KPROBES [=y]
> >>         |--- HAVE_KPROBES_ON_FTRACE [=n]
> >>         |--- DYNAMIC_FTRACE_WITH_REGS [=n]
> >>                ^----- FTRACE [=y]
> >>                  |--- DYNAMIC_FTRACE [=y]
> >>                  |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
> >> 
> >> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
> >> return the same address as the probe target.
> >> 
> >> However, when KPROBES_ON_FTRACE=n, the current implementation
> >> returns -EINVAL after calling check_ftrace_location(), causing
> >> the validation to fail.
> > 
> > This is a feature not a bug.
> > 
> > The reason is if you put a kprobe on a ftrace location, it can cause ftrace
> > to trigger a bug, as kprobes will modify the location and ftrace will see
> > something it doesn't expect and think the system is corrupted. We don't want
> > that either.
> > 
> > If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
> > controlled by ftrace, it had better fail!
> > 
> > NAK
> > 
> > -- Steve
> 
> Thanks for your clear explanation. I will look into other approaches
> that work with this configuration.
> 

Can you check the code under arch/<your machine>/ implements the
kprobe_ftrace_handler() correctly? if so, it should be enabled automatically.

Thank you,


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE

[qingwei hu]

> On Dec 8, 2025, at 15:19, Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> On Mon, 8 Dec 2025 14:54:33 +0800
> qingwei hu <huqingwei.kernel@gmail.com> wrote:
> 
>> 
>> 
>>> 2025年12月5日 23:08，Steven Rostedt <rostedt@goodmis.org> 写道：
>>> 
>>> On Fri,  5 Dec 2025 17:29:33 +0800
>>> "qingwei.hu" <qingwei.hu@bytedance.com> wrote:
>>> 
>>>> From: Qingwei Hu <qingwei.hu@bytedance.com>
>>>> 
>>>> There is a possible configuration dependency:
>>>> 
>>>> KPROBES_ON_FTRACE [=n]
>>>>      ^----- KPROBES [=y]
>>>>        |--- HAVE_KPROBES_ON_FTRACE [=n]
>>>>        |--- DYNAMIC_FTRACE_WITH_REGS [=n]
>>>>               ^----- FTRACE [=y]
>>>>                 |--- DYNAMIC_FTRACE [=y]
>>>>                 |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
>>>> 
>>>> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
>>>> return the same address as the probe target.
>>>> 
>>>> However, when KPROBES_ON_FTRACE=n, the current implementation
>>>> returns -EINVAL after calling check_ftrace_location(), causing
>>>> the validation to fail.
>>> 
>>> This is a feature not a bug.
>>> 
>>> The reason is if you put a kprobe on a ftrace location, it can cause ftrace
>>> to trigger a bug, as kprobes will modify the location and ftrace will see
>>> something it doesn't expect and think the system is corrupted. We don't want
>>> that either.
>>> 
>>> If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
>>> controlled by ftrace, it had better fail!
>>> 
>>> NAK
>>> 
>>> -- Steve
>> 
>> Thanks for your clear explanation. I will look into other approaches
>> that work with this configuration.
>> 
> 
> Can you check the code under arch/<your machine>/ implements the
> kprobe_ftrace_handler() correctly? if so, it should be enabled automatically.
> 
> Thank you,
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

I am working on the risc-v that have removed support for KPROBE_ON_FTRACE and
probe_ftrace_hander() since 7caa9765465f60 and 3308172276db5d.

As 7caa9765465f60 commit say KPROBES_ON_FTRACE will be supplanted by FPROBES,
I have tested and verified this feature's usability. In this situation, may
be we should use fprobe instead of kprobe.

But in the latest version, risc-v adjusts ftrace location to an offset 4 byte
from the function symbol. So it will kprobe successfully in the beginning of
the symbol while not support KPROBE_ON_FTRACE. I am not sure whether this
arrangement is consistent with Steve’s explanation. If the adjustment is valid,
I can temporarily backport the adjust function in order to support kprobe.