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 correctness 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 trying to fix this unreliable check deep in the unwinding
path, simply remove it. The iteration is already safe from crashes because
unwind_next_frame() holds RCU and rethook_node structures are RCU-freed;
even if the iteration goes off the rails and returns invalid information,
it will not crash. Callers that require consistency must provide a safe
context themselves.
Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Tengda Wu <[email protected]>
---
v4: Also update the function description in the comment.
v3:
https://lore.kernel.org/all/[email protected]/
v2:
https://lore.kernel.org/all/[email protected]/
v1:
https://lore.kernel.org/all/[email protected]/
kernel/trace/rethook.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 5a8bdf88999a..1e7fdebe3cd5 100644
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -233,9 +233,10 @@ NOKPROBE_SYMBOL(__rethook_find_ret_addr);
*
* Find the correct return address modified by a rethook on @tsk in unsigned
* long type.
- * The @tsk must be 'current' or a task which is not running. @frame is a hint
- * to get the currect return address - which is compared with the
- * rethook::frame field. The @cur is a loop cursor for searching the
+ * @tsk can be any task (any state). If not 'current', the result may be
+ * unreliable. Callers requiring reliability must ensure a safe context.
+ * @frame is a hint to get the correct return address - which is compared with
+ * the rethook::frame field. The @cur is a loop cursor for searching the
* kretprobe return addresses on the @tsk. The '*@cur' should be NULL at the
* first call, but '@cur' itself must NOT NULL.
*
@@ -250,9 +251,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
