Skip to contentSkip to conversation

arm64: fix infinite stacktrace

PATCH New Mikulas Patocka 4 participants 8 messages Jun 14, 2018 – Jul 12, 2018 arm64

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org
arch/arm64/kernel/stacktrace.c +3 -0
Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp <= fp)
+		return -EINVAL;
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	if (tsk->ret_stack &&
 			(frame->pc == (unsigned long)return_to_handler)) {
reviewer
#1 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Hi,

Quoted 17 lines · Mikulas Patocka

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90
Quoted 5 lines

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org

Given this can only occur when there's a corrupted stack (where a frame record points to itself), I'm not sure this requires a cc stable.

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 61

Quoted 11 lines

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c +3 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp <= fp)
+		return -EINVAL;
+

Dave Martin had a series [1] which addressed this along with a number of other cases where stack traces might not terminate.

Dave, do you plan to respin that?

Thanks,
Mark.

[1] https://lkml.kernel.org/r/1524503223-17576-1-git-send-email-Dave.Martin at arm.com

#2 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Hi all,

Quoted 35 lines · Mark Rutland
Quoted 17 lines · Mikulas Patocka

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90
Quoted 5 lines

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org

Given this can only occur when there's a corrupted stack (where a frame record points to itself), I'm not sure this requires a cc stable.

Quoted 8 lines

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c +0 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
Quoted 8 lines

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 61

Quoted 3 lines
arch/arm64/kernel/stacktrace.c +3 -0
+	if (frame->fp <= fp)
+		return -EINVAL;
+

Dave Martin had a series [1] which addressed this along with a number of other cases where stack traces might not terminate.

Dave, do you plan to respin that?

I'd be interested in an update on that; we clearly should be fixing this in one way or another.

Mikulus -- would you be able to test and/or review it, please?

Will

#3 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Quoted 45 lines · Will Deacon

Hi all,

Quoted 42 lines · Mark Rutland
Quoted 17 lines · Mikulas Patocka

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90
Quoted 5 lines

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org

Given this can only occur when there's a corrupted stack (where a frame record points to itself), I'm not sure this requires a cc stable.

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 61

Quoted 11 lines

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c +3 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp <= fp)
+		return -EINVAL;
+

Dave Martin had a series [1] which addressed this along with a number of other cases where stack traces might not terminate.

Quoted 6 lines
Quoted 1 line

Dave, do you plan to respin that?

I'd be interested in an update on that; we clearly should be fixing this in one way or another.

Mikulus -- would you be able to test and/or review it, please?

My patch was arguably over-engineered, and broken in some way connected with SDEI. Unfortunately I've had too much other stuff to do...

I could take another look, but it may take time to get to it.

Alternatively, if someone wants to pick up my patch and take it forward, I'm happy to comment.

Cheers
---Dave

#4 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Quoted 56 lines · Dave Martin
Quoted 45 lines · Will Deacon

Hi all,

Quoted 42 lines · Mark Rutland
Quoted 17 lines · Mikulas Patocka

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90
Quoted 5 lines

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org

Given this can only occur when there's a corrupted stack (where a frame record points to itself), I'm not sure this requires a cc stable.

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 61

Quoted 11 lines

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c +3 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp <= fp)
+		return -EINVAL;
+

Dave Martin had a series [1] which addressed this along with a number of other cases where stack traces might not terminate.

Quoted 6 lines
Quoted 1 line

Dave, do you plan to respin that?

I'd be interested in an update on that; we clearly should be fixing this in one way or another.

Mikulus -- would you be able to test and/or review it, please?

My patch was arguably over-engineered, and broken in some way connected with SDEI. Unfortunately I've had too much other stuff to do...

Quoted 7 lines

I could take another look, but it may take time to get to it.

Alternatively, if someone wants to pick up my patch and take it forward, I'm happy to comment.

Cheers
---Dave

Or - what about this patch? It doesn't guarantee that infinite loop won't happen, but it at least catches the most obvious loop on the same address.

From: Mikulas Patocka <[email protected]>
Subject: [PATCH] arm64: fix infinite stacktrace

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org
---
 arch/arm64/kernel/stacktrace.c |    3 +++
 1 file changed, 3 insertions(+)
arch/arm64/kernel/stacktrace.c +3 -0
Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp == fp)
+		return -EINVAL;
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	if (tsk->ret_stack &&
 			(frame->pc == (unsigned long)return_to_handler)) {
#5 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Quoted 46 lines · Mikulas Patocka
Quoted 5 lines · Dave Martin

Alternatively, if someone wants to pick up my patch and take it forward, I'm happy to comment.

Cheers
---Dave

Or - what about this patch? It doesn't guarantee that infinite loop won't happen, but it at least catches the most obvious loop on the same address.

From: Mikulas Patocka <[email protected]>
Subject: [PATCH] arm64: fix infinite stacktrace

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org
---
 arch/arm64/kernel/stacktrace.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 60

Quoted 5 lines
arch/arm64/kernel/stacktrace.c +2 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp == fp)
+		return -EINVAL;

I've already queued your previous patch using '<=' (it's in -next). If you want to change it, please send patches on top of the arm64 for-next/core branch.

Will

#6 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Quoted 50 lines · Will Deacon
Quoted 49 lines · Mikulas Patocka
Quoted 5 lines · Dave Martin

Alternatively, if someone wants to pick up my patch and take it forward, I'm happy to comment.

Cheers
---Dave

Or - what about this patch? It doesn't guarantee that infinite loop won't happen, but it at least catches the most obvious loop on the same address.

From: Mikulas Patocka <[email protected]>
Subject: [PATCH] arm64: fix infinite stacktrace

I've got this infinite stacktrace when debugging another problem:

[  908.795225] INFO: rcu_preempt detected stalls on CPUs/tasks:
[  908.796176]  1-...!: (1 GPs behind) idle=952/1/4611686018427387904 softirq=1462/1462 fqs=355
[  908.797692]  2-...!: (1 GPs behind) idle=f42/1/4611686018427387904 softirq=1550/1551 fqs=355
[  908.799189]  (detected by 0, t=2109 jiffies, g=130, c=129, q=235)
[  908.800284] Task dump for CPU 1:
[  908.800871] kworker/1:1     R  running task        0    32      2 0x00000022
[  908.802127] Workqueue: writecache-writeabck writecache_writeback [dm_writecache]
[  908.820285] Call trace:
[  908.824785]  __switch_to+0x68/0x90
[  908.837661]  0xfffffe00603afd90
[  908.844119]  0xfffffe00603afd90
[  908.850091]  0xfffffe00603afd90
[  908.854285]  0xfffffe00603afd90
[  908.863538]  0xfffffe00603afd90
[  908.865523]  0xfffffe00603afd90

The machine just locked up and kept on printing the same line over and over again. This patch fixes it.

Cc stable at vger.kernel.org
---
 arch/arm64/kernel/stacktrace.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c +0 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
Quoted 8 lines

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 60

Quoted 2 lines
arch/arm64/kernel/stacktrace.c +2 -0
+	if (frame->fp == fp)
+		return -EINVAL;

I've already queued your previous patch using '<=' (it's in -next). If you want to change it, please send patches on top of the arm64 for-next/core branch.

Will

I don't know - is this function supposed to backtrace against more discontiguous stacks? If yes, then the patch with (frame->fp <= fp) is incorrect. If no, then the condition (frame->fp <= fp) could be there.

Mikulas

#7 lore raw

PATCH 1/1 arm64: fix infinite stacktrace

Quoted 17 lines · Mikulas Patocka
Quoted 16 lines · Will Deacon

arch/arm64/kernel/stacktrace.c int notrace unwind_frame(struct task_str · line 60

Quoted 10 lines · Mikulas Patocka

Index: linux-2.6/arch/arm64/kernel/stacktrace.c
===================================================================

arch/arm64/kernel/stacktrace.c +2 -0
@@ -56,6 +56,9 @@ int notrace unwind_frame(struct task_str
 	frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
 	frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
 
+	if (frame->fp == fp)
+		return -EINVAL;

I've already queued your previous patch using '<=' (it's in -next). If you want to change it, please send patches on top of the arm64 for-next/core branch.

Quoted 5 lines
Quoted 1 line

Will

I don't know - is this function supposed to backtrace against more discontiguous stacks? If yes, then the patch with (frame->fp <= fp) is incorrect. If no, then the condition (frame->fp <= fp) could be there.

Ah yes, your original patch breaks with irqstacks, so I'll need to revert it. Using '==' feels like a big hack to me.

Will