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

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

[Tengda Wu]

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.

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.

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

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

[Peter Zijlstra]

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
> 

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

[Tengda Wu]

On 2026/6/9 15:14, Peter Zijlstra wrote:
> 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>
> 

Thank you for the review and suggestions, Peter.

I have incorporated your feedback into v3. The patch has been sent out.
https://lore.kernel.org/all/20260609084953.901576-1-wutengda@huaweicloud.com/

Best regards,
Tengda