Skip to contentSkip to conversation

Fix stack alignment when processing backtraces

PATCH In review Jason Gunthorpe 2 participants 4 messages Oct 18, 2016 – Oct 31, 2016 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix stack alignment when processing backtraces

The dumpstm helper within c_backtrace pushed 5 dwords onto the stack causing the stack to become unaligned and then calls printk. This causes memory corruption in the kernel which assumes AAPCS calling convention.

Since this bit of asm doesn't use the standard prologue just add another register to restore alignment.

Fixes 7ab3f8d595a1b ("[ARM] Add ability to dump exception stacks to kernel backtraces")
---
 arch/arm/lib/backtrace.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

In my case the kernel was hitting a WARN_ON during boot and then reliably failed to start the compiled-in initramfs.

I'm inferring that the stack misalignment caused some kind of memory corruption which wiped out the unpacked initramfs.

Saw with gcc 5.4.0 on a kirkwood armv5te

arch/arm/lib/backtrace.S +3 -2
index fab5a50503ae..25e1cce19991 100644
@@ -116,7 +116,8 @@ ENDPROC(c_backtrace)
 #define reg   r5
 #define stack r6
 
-.Ldumpstm:	stmfd	sp!, {instr, reg, stack, r7, lr}
+	        /* Must maintain 8 byte stack alignment */
+.Ldumpstm:	stmfd	sp!, {r3, instr, reg, stack, r7, lr}
 		mov	stack, r0
 		mov	instr, r1
 		mov	reg, #10
@@ -140,7 +141,7 @@ ENDPROC(c_backtrace)
 		teq	r7, #0
 		adrne	r0, .Lcr
 		blne	printk
-		ldmfd	sp!, {instr, reg, stack, r7, pc}
+		ldmfd	sp!, {r3, instr, reg, stack, r7, pc}
 
 .Lfp:		.asciz	" r%d:%08x%s"
 .Lcr:		.asciz	"\n"
Signature · 1 line
2.1.4
#1 lore raw
Quoted 13 lines · Jason Gunthorpe

The dumpstm helper within c_backtrace pushed 5 dwords onto the stack causing the stack to become unaligned and then calls printk. This causes memory corruption in the kernel which assumes AAPCS calling convention.

Since this bit of asm doesn't use the standard prologue just add another register to restore alignment.

Fixes 7ab3f8d595a1b ("[ARM] Add ability to dump exception stacks to kernel backtraces")
 arch/arm/lib/backtrace.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 7 lines

In my case the kernel was hitting a WARN_ON during boot and then reliably failed to start the compiled-in initramfs.

I'm inferring that the stack misalignment caused some kind of memory corruption which wiped out the unpacked initramfs.

Saw with gcc 5.4.0 on a kirkwood armv5te

Since there are no comments, I will send this to RMK's patch system..

Thanks,
Jason

Quoted 22 lines
arch/arm/lib/backtrace.S +4 -2
index fab5a50503ae..25e1cce19991 100644
+++ b/arch/arm/lib/backtrace.S
@@ -116,7 +116,8 @@ ENDPROC(c_backtrace)
 #define reg   r5
 #define stack r6
 
-.Ldumpstm:	stmfd	sp!, {instr, reg, stack, r7, lr}
+	        /* Must maintain 8 byte stack alignment */
+.Ldumpstm:	stmfd	sp!, {r3, instr, reg, stack, r7, lr}
 		mov	stack, r0
 		mov	instr, r1
 		mov	reg, #10
@@ -140,7 +141,7 @@ ENDPROC(c_backtrace)
 		teq	r7, #0
 		adrne	r0, .Lcr
 		blne	printk
-		ldmfd	sp!, {instr, reg, stack, r7, pc}
+		ldmfd	sp!, {r3, instr, reg, stack, r7, pc}
 
 .Lfp:		.asciz	" r%d:%08x%s"
 .Lcr:		.asciz	"\n"
#2 lore raw
In reply to Jason Gunthorpe
Quoted 37 lines · Jason Gunthorpe

The dumpstm helper within c_backtrace pushed 5 dwords onto the stack causing the stack to become unaligned and then calls printk. This causes memory corruption in the kernel which assumes AAPCS calling convention.

Since this bit of asm doesn't use the standard prologue just add another register to restore alignment.

Fixes 7ab3f8d595a1b ("[ARM] Add ability to dump exception stacks to kernel backtraces")
---
 arch/arm/lib/backtrace.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

In my case the kernel was hitting a WARN_ON during boot and then reliably failed to start the compiled-in initramfs.

I'm inferring that the stack misalignment caused some kind of memory corruption which wiped out the unpacked initramfs.

Saw with gcc 5.4.0 on a kirkwood armv5te

arch/arm/lib/backtrace.S +2 -1
index fab5a50503ae..25e1cce19991 100644
@@ -116,7 +116,8 @@ ENDPROC(c_backtrace)
 #define reg   r5
 #define stack r6
 
-.Ldumpstm:	stmfd	sp!, {instr, reg, stack, r7, lr}
+	        /* Must maintain 8 byte stack alignment */
+.Ldumpstm:	stmfd	sp!, {r3, instr, reg, stack, r7, lr}
 		mov	stack, r0
 		mov	instr, r1
 		mov	reg, #10

arch/arm/lib/backtrace.S ENDPROC(c_backtrace) · line 144

Quoted 5 lines
arch/arm/lib/backtrace.S +1 -1
@@ -140,7 +141,7 @@ ENDPROC(c_backtrace)
 		teq	r7, #0
 		adrne	r0, .Lcr
 		blne	printk
-		ldmfd	sp!, {instr, reg, stack, r7, pc}
+		ldmfd	sp!, {r3, instr, reg, stack, r7, pc}

I'd prefer r8 to get used rather than r3, as it makes it look like r3 is somehow required to be preserved when that's not the case. Makes the code slightly more difficult to understand.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#3 lore raw
Quoted 8 lines · Russell King - ARM Linux
Quoted 8 lines

-.Ldumpstm: stmfd sp!, {instr, reg, stack, r7, lr}

arch/arm/lib/backtrace.S +2 -0
+	        /* Must maintain 8 byte stack alignment */
+.Ldumpstm:	stmfd	sp!, {r3, instr, reg, stack, r7, lr}
 		mov	stack, r0
 		mov	instr, r1
 		mov	reg, #10
@@ -140,7 +141,7 @@ ENDPROC(c_backtrace)
 		teq	r7, #0
Quoted 8 lines

arch/arm/lib/backtrace.S ENDPROC(c_backtrace) · line 144

Quoted 4 lines
arch/arm/lib/backtrace.S +1 -1
 		adrne	r0, .Lcr
 		blne	printk
-		ldmfd	sp!, {instr, reg, stack, r7, pc}
+		ldmfd	sp!, {r3, instr, reg, stack, r7, pc}

I'd prefer r8 to get used rather than r3, as it makes it look like r3 is somehow required to be preserved when that's not the case. Makes the code slightly more difficult to understand.

Sure, I will change that and send it to your tracker.

Thanks,
Jason