[PATCH v2] samples/ftrace: Fix kthread_stop() on ERR_PTR in ftrace-direct-modify

[PATCH v2] samples/ftrace: Fix kthread_stop() on ERR_PTR in ftrace-direct-modify

[Haotian Zhang]

ftrace_direct_init() assigns kthread_run()'s return value to simple_tsk
without an IS_ERR() check. When kthread_run() fails it returns
ERR_PTR(-ENOMEM), but init still returns 0, so the module loads with
simple_tsk holding an error pointer. On unload, ftrace_direct_exit()
then passes that ERR_PTR to kthread_stop(), leading to a
null-pointer-dereference.

Check the return value of kthread_run() with IS_ERR(); on failure,
unregister the ftrace direct call and propagate the error code.

Fixes: ae0cc3b7e7f5 ("ftrace/samples: Add a sample module that implements modify_ftrace_direct()")
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
Changes in v2:
  - Simplify the code as suggested by Steven Rostedt.---
---
 samples/ftrace/ftrace-direct-modify.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index da3a9f2091f5..246a8b9b22ce 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -320,9 +320,15 @@ static int __init ftrace_direct_init(void)
 	ftrace_set_filter_ip(&direct, (unsigned long) my_ip, 0, 0);
 	ret = register_ftrace_direct(&direct, my_tramp);
 
-	if (!ret)
-		simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
-	return ret;
+	if (ret)
+		return ret;
+	simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
+	if (IS_ERR(simple_tsk)) {
+		unregister_ftrace_direct(&direct, my_tramp, true);
+		return PTR_ERR(simple_tsk);
+	}
+
+	return 0;
 }
 
 static void __exit ftrace_direct_exit(void)
-- 
2.43.0

Re: [PATCH v2] samples/ftrace: Fix kthread_stop() on ERR_PTR in ftrace-direct-modify

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing IS_ERR() check on kthread_run() leads to kernel panic on module unload
--

commit 83cdbb346a20d7607e5ef97af8d7a9a9c9c57687
Author: Haotian Zhang <vulab@iscas.ac.cn>

samples/ftrace: Fix kthread_stop() on ERR_PTR in ftrace-direct-modify

This commit fixes a bug in the ftrace-direct-modify sample module where
the return value of kthread_run() was not checked with IS_ERR(). Without
this check, an initialization failure could leave an error pointer in
simple_tsk, causing a kernel panic when kthread_stop() is called during
module unload. The patch correctly propagates the error and cleans up
by unregistering the ftrace direct call on failure.

> diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
> index 1ba1927b548ee..164d9dd6fd921 100644
> --- a/samples/ftrace/ftrace-direct-modify.c
> +++ b/samples/ftrace/ftrace-direct-modify.c
> @@ -320,9 +320,15 @@ static int __init ftrace_direct_init(void)
>  	ftrace_set_filter_ip(&direct, (unsigned long) my_ip, 0, 0);
>  	ret = register_ftrace_direct(&direct, my_tramp);
>  
> -	if (!ret)
> -		simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
> -	return ret;
> +	if (ret)
> +		return ret;
> +	simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
> +	if (IS_ERR(simple_tsk)) {
> +		unregister_ftrace_direct(&direct, my_tramp, true);
> +		return PTR_ERR(simple_tsk);
> +	}
> +
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but it looks like the exact same missing
IS_ERR() check is present in samples/ftrace/ftrace-direct-multi-modify.c:

samples/ftrace/ftrace-direct-multi-modify.c:ftrace_direct_multi_init() {
    [ ... ]
	ret = register_ftrace_direct(&direct, my_tramp);

	if (!ret)
		simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
	return ret;
}

If kthread_run() fails here, it returns an ERR_PTR which is assigned to
simple_tsk, and the module still successfully loads since the function
returns 0.

When the module is unloaded, kthread_stop() is called on the ERR_PTR, which
dereferences it and causes a kernel panic.

Should this same fix be applied to ftrace_direct_multi_init() in
ftrace-direct-multi-modify.c as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826015034.10755-1-vulab@iscas.ac.cn?part=1