Re: [PATCH v2] rethook: Remove the running task check in rethook_find_ret_addr()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Jun 09, 2026 at 08:57:28AM +0800, Tengda Wu wrote:
> The current check in rethook_find_ret_addr() prevents obtaining a return
> address when the target task is marked as running. However, this condition
> is both insufficient for safety and unnecessary for its intended purpose.

Depends on what safety means. If safety means not crashing, it is
entirely superfluous. If safety means correctness, then yes, it is
insufficient.

> The check is inherently racy: a task can begin running on another CPU
> immediately after task_is_running() returns false, potentially leading to
> concurrent modification of rethook data structures while the iteration is
> in progress.
> 
> Rather than attempting to fix this unreliable check deep in the unwinding
> path, remove it entirely. Callers that require consistency are expected
> to provide a safe context.

Perhaps also note that unwind_next() will hold RCU and the rethook_node
things are RCU freed, so while the iteration might go off the rails and
return invalid information, it will not crash.


> Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>

With clarifications:

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
> v2: Remove the running task check.
> v1: https://lore.kernel.org/all/20260525132253.1889726-1-wutengda@huaweicloud.com/
> 
>  kernel/trace/rethook.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> index 5a8bdf88999a..f70f11bc6c91 100644
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -250,9 +250,6 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
>  	if (WARN_ON_ONCE(!cur))
>  		return 0;
>  
> -	if (tsk != current && task_is_running(tsk))
> -		return 0;
> -
>  	do {
>  		ret = __rethook_find_ret_addr(tsk, cur);
>  		if (!ret)
> -- 
> 2.34.1
>