Skip to contentSkip to conversation

ARM: fix v7-M signal return

PATCH New Rabin Vincent 2 participants 6 messages Apr 21, 2014 – May 5, 2014 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: fix v7-M signal return

According to the ARM ARM, the behaviour is UNDPREDICTABLE if the PC read from the exception return stack is not half word aligned. See the pseudo code for ExceptionReturn() and PopStack().

The signal handler's address has the bit 0 set, and setup_return() directly writes this to regs->ARM_pc. Mask out bit 0 before the exception return to get predictable behaviour.

---
 arch/arm/kernel/entry-header.S | 1 +
 1 file changed, 1 insertion(+)
arch/arm/kernel/entry-header.S +1 -0
index 1420725..ef72f4b 100644
@@ -133,6 +133,7 @@
 	biceq	r5, V7M_xPSR_FRAMEPTRALIGN
 
 	@ write basic exception frame
+	bic	r4, r4, #1
 	stmdb	r2!, {r1, r3-r5}
 	ldmia	sp, {r1, r3-r5}
 	.if	\ret_r0
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 ARM: fix v7-M signal return

Quoted 18 lines · Rabin Vincent

According to the ARM ARM, the behaviour is UNDPREDICTABLE if the PC read from the exception return stack is not half word aligned. See the pseudo code for ExceptionReturn() and PopStack().

The signal handler's address has the bit 0 set, and setup_return() directly writes this to regs->ARM_pc. Mask out bit 0 before the exception return to get predictable behaviour.

---
 arch/arm/kernel/entry-header.S | 1 +
 1 file changed, 1 insertion(+)
arch/arm/kernel/entry-header.S +0 -0
index 1420725..ef72f4b 100644

arch/arm/kernel/entry-header.S line 139

Quoted 7 lines
arch/arm/kernel/entry-header.S +1 -0
@@ -133,6 +133,7 @@
 	biceq	r5, V7M_xPSR_FRAMEPTRALIGN
 
 	@ write basic exception frame
+	bic	r4, r4, #1
 	stmdb	r2!, {r1, r3-r5}
 	ldmia	sp, {r1, r3-r5}
 	.if	\ret_r0

This is a valid fix, but it seems on my efm32 the unpredictable behaviour is to just discard the LSB. How did you find that? Is it an issue on your machine? Which cpu are you using?

I'd like to have the instruction clearing the thumb bit above the comment about the basic exception frame and please add a comment for your instruction, too.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#2 lore raw

PATCH 1/1 ARM: fix v7-M signal return

Quoted 3 lines · Uwe Kleine-K?nig

This is a valid fix, but it seems on my efm32 the unpredictable behaviour is to just discard the LSB. How did you find that? Is it an issue on your machine? Which cpu are you using?

I'm running this on QEMU. Here is an old qemu-devel thread on this topic if you are interested:
http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg00158.html

Quoted 3 lines

I'd like to have the instruction clearing the thumb bit above the comment about the basic exception frame and please add a comment for your instruction, too.

OK, here is a v2 with those changes:

8<------------------

Quoted 1 line

From 4aa76f95a6ecf781eec89dba8a3884e5e4339182 Mon Sep 17 00:00:00 2001

From: Rabin Vincent <[email protected]>
Date: Sat, 3 May 2014 19:27:09 +0200
Subject: [PATCHv2] ARM: fix v7-M signal return

According to the ARM ARM, the behaviour is UNDPREDICTABLE if the PC read from the exception return stack is not half word aligned. See the pseudo code for ExceptionReturn() and PopStack().

The signal handler's address has the bit 0 set, and setup_return() directly writes this to regs->ARM_pc. Mask out bit 0 before the exception return to get predictable behaviour.

---
 arch/arm/kernel/entry-header.S | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/kernel/entry-header.S +3 -0
index 1420725..743dff6 100644
@@ -132,6 +132,9 @@
 	orrne	r5, V7M_xPSR_FRAMEPTRALIGN
 	biceq	r5, V7M_xPSR_FRAMEPTRALIGN
 
+	@ ensure bit 0 is cleared in the PC
+	bic	r4, r4, #1
+
 	@ write basic exception frame
 	stmdb	r2!, {r1, r3-r5}
 	ldmia	sp, {r1, r3-r5}
Signature · 1 line
1.9.2
#3 lore raw

PATCH 1/1 ARM: fix v7-M signal return

Hello Rabin,

Quoted 8 lines · Rabin Vincent
Quoted 3 lines · Uwe Kleine-K?nig

This is a valid fix, but it seems on my efm32 the unpredictable behaviour is to just discard the LSB. How did you find that? Is it an issue on your machine? Which cpu are you using?

I'm running this on QEMU. Here is an old qemu-devel thread on this topic if you are interested:
http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg00158.html

I'm interested in your setup and (if applicable) additional kernel patches.

Quoted 30 lines
Quoted 3 lines

I'd like to have the instruction clearing the thumb bit above the comment about the basic exception frame and please add a comment for your instruction, too.

OK, here is a v2 with those changes:

8<------------------
From 4aa76f95a6ecf781eec89dba8a3884e5e4339182 Mon Sep 17 00:00:00 2001
From: Rabin Vincent <[email protected]>
Date: Sat, 3 May 2014 19:27:09 +0200
Subject: [PATCHv2] ARM: fix v7-M signal return

According to the ARM ARM, the behaviour is UNDPREDICTABLE if the PC read from the exception return stack is not half word aligned. See the pseudo code for ExceptionReturn() and PopStack().

The signal handler's address has the bit 0 set, and setup_return() directly writes this to regs->ARM_pc. Mask out bit 0 before the exception return to get predictable behaviour.

---
 arch/arm/kernel/entry-header.S | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/kernel/entry-header.S +0 -0
index 1420725..743dff6 100644

arch/arm/kernel/entry-header.S line 135

Quoted 4 lines
arch/arm/kernel/entry-header.S +1 -0
@@ -132,6 +132,9 @@
 	orrne	r5, V7M_xPSR_FRAMEPTRALIGN
 	biceq	r5, V7M_xPSR_FRAMEPTRALIGN
 
+	@ ensure bit 0 is cleared in the PC

Maybe add: , otherwise behaviour is unpredictable

arch/arm/kernel/entry-header.S line 136

Quoted 1 line
arch/arm/kernel/entry-header.S +1 -0
+	bic	r4, r4, #1

I just notice that the coding style is inconsitent here, the instructions above don't repeat the dest register. Can you please make this "bic r4, #1", too?

With these two changes you can have my ack.

What happens on qemu without this fix? If it crashes I'd suggest to get this patch into 3.15 with a stable annotation. If not I think 3.16-rc1 is enough.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#4 lore raw

PATCH 1/1 ARM: fix v7-M signal return

Quoted 6 lines · Uwe Kleine-K?nig
Quoted 3 lines · Rabin Vincent

I'm running this on QEMU. Here is an old qemu-devel thread on this topic if you are interested:
http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg00158.html

I'm interested in your setup and (if applicable) additional kernel patches.

No kernel patches are needed other than the ones I already posted. My QEMU patches for EFM32 support are needed. I put together a small README here: https://github.com/rabinv/qemu-efm32-tools

Quoted 9 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+	@ ensure bit 0 is cleared in the PC

Maybe add: , otherwise behaviour is unpredictable

Review

Quoted 1 line
(quoted diff) +1 -0
+	bic	r4, r4, #1

I just notice that the coding style is inconsitent here, the instructions above don't repeat the dest register. Can you please make this "bic r4, #1", too?

With these two changes you can have my ack.

v3 below.

Quoted 3 lines

What happens on qemu without this fix? If it crashes I'd suggest to get this patch into 3.15 with a stable annotation. If not I think 3.16-rc1 is enough.

User space processes crash because qemu's emulation of the CPU does not discard the zero bit.

8<--------------------

Quoted 1 line

From 577b0c0e15057f58f86996da7f363c8a608c389f Mon Sep 17 00:00:00 2001

From: Rabin Vincent <[email protected]>
Date: Sat, 3 May 2014 19:27:09 +0200
Subject: [PATCHv3] ARM: fix v7-M signal return

According to the ARM ARM, the behaviour is UNDPREDICTABLE if the PC read from the exception return stack is not half word aligned. See the pseudo code for ExceptionReturn() and PopStack().

The signal handler's address has the bit 0 set, and setup_return() directly writes this to regs->ARM_pc. Mask out bit 0 before the exception return to get predictable behaviour.

---
 arch/arm/kernel/entry-header.S | 4 ++++
 1 file changed, 4 insertions(+)
arch/arm/kernel/entry-header.S +4 -0
index 1420725..efb208d 100644
@@ -132,6 +132,10 @@
 	orrne	r5, V7M_xPSR_FRAMEPTRALIGN
 	biceq	r5, V7M_xPSR_FRAMEPTRALIGN
 
+	@ ensure bit 0 is cleared in the PC, otherwise behaviour is
+	@ unpredictable
+	bic	r4, #1
+
 	@ write basic exception frame
 	stmdb	r2!, {r1, r3-r5}
 	ldmia	sp, {r1, r3-r5}
Signature · 1 line
2.0.0.rc0
#5 lore raw

PATCH 1/1 ARM: fix v7-M signal return

Hello Rabin,

Quoted 31 lines · Rabin Vincent
Quoted 6 lines · Uwe Kleine-K?nig
Quoted 3 lines · Rabin Vincent

I'm running this on QEMU. Here is an old qemu-devel thread on this topic if you are interested:
http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg00158.html

I'm interested in your setup and (if applicable) additional kernel patches.

No kernel patches are needed other than the ones I already posted. My QEMU patches for EFM32 support are needed. I put together a small README here: https://github.com/rabinv/qemu-efm32-tools

Quoted 9 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+	@ ensure bit 0 is cleared in the PC

Maybe add: , otherwise behaviour is unpredictable

Review

Quoted 1 line
(quoted diff) +1 -0
+	bic	r4, r4, #1

I just notice that the coding style is inconsitent here, the instructions above don't repeat the dest register. Can you please make this "bic r4, #1", too?

With these two changes you can have my ack.

v3 below.

Quoted 3 lines

What happens on qemu without this fix? If it crashes I'd suggest to get this patch into 3.15 with a stable annotation. If not I think 3.16-rc1 is enough.

User space processes crash because qemu's emulation of the CPU does not discard the zero bit.

Quoted 7 lines

8<--------------------
From 577b0c0e15057f58f86996da7f363c8a608c389f Mon Sep 17 00:00:00 2001
From: Rabin Vincent <[email protected]>
Date: Sat, 3 May 2014 19:27:09 +0200
Subject: [PATCHv3] ARM: fix v7-M signal return

According to the ARM ARM, the behaviour is UNDPREDICTABLE if the PC read

s/UNDPR/UNPR/

Quoted 6 lines

from the exception return stack is not half word aligned. See the pseudo code for ExceptionReturn() and PopStack().

The signal handler's address has the bit 0 set, and setup_return() directly writes this to regs->ARM_pc. Mask out bit 0 before the exception return to get predictable behaviour.

Actual machines seem to behave just fine (i.e. discard this bit) but qemu's emulation doesn't and makes processes crash.

Fixes 19c4d593f0b4 ("ARM: ARMv7-M: Add support for exception handling")
Cc stable at kernel.org

----
Can you please put this in Russell's patch tracker.

Maybe pc in the saved struct regs should get bit 0 set during exception entry for consistency?! (That would be a separate patch of course.)

Thanks
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |