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

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

[Haotian Zhang]

ftrace_direct_multi_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_multi_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: e1067a07cfbc ("ftrace/samples: Add module to test multi direct modify interface")
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-multi-modify.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
index 8f7986d698d8..70c2263d839c 100644
--- a/samples/ftrace/ftrace-direct-multi-modify.c
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -364,9 +364,15 @@ static int __init ftrace_direct_multi_init(void)
 
 	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_multi_exit(void)
-- 
2.43.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes an invalid pointer dereference resulting from a kthread_run() failure in ftrace-direct-multi-modify.c, but leaves the identical vulnerability unpatched in ftrace-direct-modify.c.
--

commit 7f181703b20421e482d5701113538a5a801d8141
Author: Haotian Zhang <vulab@iscas.ac.cn>

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

This commit fixes a bug in ftrace-direct-multi-modify.c where a failed
kthread_run() returns an error pointer that is later passed to kthread_stop()
during module unload. It adds proper error handling to unregister the direct
call and propagate the error.

> diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
> --- a/samples/ftrace/ftrace-direct-multi-modify.c
> +++ b/samples/ftrace/ftrace-direct-multi-modify.c
> @@ -364,9 +364,15 @@ static int __init ftrace_direct_multi_init(void)
>  
>  	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 isn't a bug introduced by this patch, but does 
samples/ftrace/ftrace-direct-modify.c also need this exact same fix?

Looking at ftrace_direct_init() in samples/ftrace/ftrace-direct-modify.c, it
contains the identical unpatched pattern:

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

If kthread_run() fails under memory pressure, it returns an ERR_PTR. Because
the preceding ret is 0, the initialization function still returns success,
causing the module to load with simple_tsk holding an error pointer. 

When the module is unloaded, ftrace_direct_exit() will pass this ERR_PTR
directly to kthread_stop(), which can result in an invalid pointer
dereference and oops the kernel.

Should this same error handling update be applied to ftrace-direct-modify.c?

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