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 <[email protected]>

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/[email protected]?part=1

Reply via email to