Re: [PATCH] arm64: break while loop if task had been rescheduled

From: tengfeif
Date: Wed May 29 2019 - 22:46:14 EST


On 2019-05-24 18:41, Mark Rutland wrote:
On Tue, May 21, 2019 at 05:20:04PM +0800, Tengfei Fan wrote:
While printing a task's backtrace and this task isn't
current task, it is possible that task's fp and fp+8
have the same value, so cannot break the while loop.
This can break while loop if this task had been
rescheduled during print this task's backtrace.

There are a few cases where backtracing can get stuck in an infinite
loop. I'd attempted to address that more generally in my
arm64/robust-stacktrace branch [1].

Looking at tsk->state here is inherently racy, and doesn't solve the
general case, so I'd prefer to avoid that.

Do my patches help you here? If so, I'm happy to rebase those to
v5.2-rc1 and repost.

I think your arm64/robust-stacktrace branch [1] can cover my issue, please
rebase and reposet

Thanks,
Tengfei Fan


Thanks,
Mark.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/robust-stacktrace


Signed-off-by: Tengfei Fan <tengfeif@xxxxxxxxxxxxxx>
---
arch/arm64/kernel/traps.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 2975598..9df6e02 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -103,6 +103,9 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
{
struct stackframe frame;
int skip = 0;
+ long cur_state = 0;
+ unsigned long cur_sp = 0;
+ unsigned long cur_fp = 0;

pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);

@@ -127,6 +130,9 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
*/
frame.fp = thread_saved_fp(tsk);
frame.pc = thread_saved_pc(tsk);
+ cur_state = tsk->state;
+ cur_sp = thread_saved_sp(tsk);
+ cur_fp = frame.fp;
}
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
frame.graph = 0;
@@ -134,6 +140,23 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)

printk("Call trace:\n");
do {
+ if (tsk != current && (cur_state != tsk->state
+ /*
+ * We would not be printing backtrace for the task
+ * that has changed state from uninterruptible to
+ * running before hitting the do-while loop but after
+ * saving the current state. If task is in running
+ * state before saving the state, then we may print
+ * wrong call trace or end up in infinite while loop
+ * if *(fp) and *(fp+8) are same. While the situation
+ * will stop print when that task schedule out.
+ */
+ || cur_sp != thread_saved_sp(tsk)
+ || cur_fp != thread_saved_fp(tsk))) {
+ printk("The task:%s had been rescheduled!\n",
+ tsk->comm);
+ break;
+ }
/* skip until specified stack frame */
if (!skip) {
dump_backtrace_entry(frame.pc);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project