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

[sashiko-bot@kernel.org]

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