[PATCH 0/2] arm: alignment trap tweaks

[PATCH 0/2] arm: alignment trap tweaks

[Robin Murphy]

Hi all,

Revisiting the alignment trap again thanks to an obscure corner case
with NEON alignment hints - patch 1 is a repost from a while back as a
ping, because modern userspaces really should just get a resounding
"Don't do that!" if they manage to step outside the architecture.
Patch 2 is the fix for cases where fixup does need to be turned back on
for whatever reason.

The following testcase compiled with -marm illustrates the problem:
with fixup enabled, when the VLD1 instruction with the alignment hint
set faults by performing a misaligned access, it fails to load the NEON
registers as expected and eventually writes back nonsense to the base
register, resulting in a misleading segfault on the next iteration.

--->8---

#include <stdint.h>
#include <stdio.h>

static uint8_t buffer[256], got[256];

int main() {
	uint8_t *base = (uint8_t *)((intptr_t)buffer + 0x7 & ~0x7);
	void *p = base, *q = got;
	int i;

	for (i=0; i<64; i++)
		base[i] = i;
	for (i=0; i<4; i++) {
		asm volatile ("vld1.8 {d0,d1}, [%0]\n vst1.8 {d0,d1}, [%1]\n"
				: "=r"(p),"=r"(q) : "0"(p),"1"(q) : "d0","d1");
		printf("unaligned: %p [%d,%d,%d,...]\n", p++, got[0], got[1], got[2]);
	}
	p = base;
	for (i=0; i<4; i++) {
		asm volatile ("vld1.8 {d0,d1}, [%0:64]\n vst1.8 {d0,d1}, [%1]\n"
				: "=r"(p),"=r"(q) : "0"(p),"1"(q) : "d0","d1");
		printf("misaligned: %p [%d,%d,%d,...]\n", p++, got[0], got[1], got[2]);
	}
	return 0;
}

--->8---

Robin Murphy (2):
  arm: SIGBUS on unsupported ARMv6 unaligned accesses
  arm: don't break misaligned NEON load/store

 arch/arm/mm/alignment.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--
1.7.9.5

[PATCH RESEND 1/2] arm: SIGBUS on unsupported ARMv6 unaligned accesses

[Robin Murphy]

This patch changes the default behaviour for userspace alignment faults
on v6 from silent fixup to SIGBUS. This only affects code that violates
the v6 unaligned access model - bad assembly/JIT code or high-level
language code in violation of the relevant language spec - which should
be corrected rather than unwittingly relying on performance-degrading
fixups. Fixup behaviour can still be controlled from boot parameters or
at runtime for compatibility with existing incorrect software.

There will be no impact for v5 and earlier.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
Cc: Russell King <linux@arm.linux.org.uk>
---

This one is already in the patch system as 7944/1, since nobody objected
to the original RFC or patch postings.
Russell: any news on whether you're happy to take this?

 arch/arm/mm/alignment.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 9240364..9a93315 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -106,10 +106,10 @@ static int safe_usermode(int new_usermode, bool warn)
 	 * making any progress.
 	 */
 	if (cpu_is_v6_unaligned() && !(new_usermode & (UM_FIXUP | UM_SIGNAL))) {
-		new_usermode |= UM_FIXUP;
+		new_usermode |= UM_SIGNAL;
 
 		if (warn)
-			printk(KERN_WARNING "alignment: ignoring faults is unsafe on this CPU.  Defaulting to fixup mode.\n");
+			printk(KERN_WARNING "alignment: ignoring faults is unsafe on this CPU.  Defaulting to signal mode.\n");
 	}
 
 	return new_usermode;
@@ -971,7 +971,7 @@ static int __init alignment_init(void)
 		cr_alignment &= ~CR_A;
 		cr_no_alignment &= ~CR_A;
 		set_cr(cr_alignment);
-		ai_usermode = safe_usermode(ai_usermode, false);
+		ai_usermode = safe_usermode(ai_usermode | UM_WARN, false);
 	}
 #endif
 
-- 
1.7.9.5

[PATCH 2/2] arm: don't break misaligned NEON load/store

[Robin Murphy]

The alignment fixup incorrectly decodes faulting ARM VLDn/VSTn
instructions as LDR/STR, leading to register corruption. Detect these
and correctly treat them as unhandled.

Reported-by: Simon Hosie <simon.hosie@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm/mm/alignment.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 9a93315..5812e0c 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -40,6 +40,7 @@
  * This code is not portable to processors with late data abort handling.
  */
 #define CODING_BITS(i)	(i & 0x0e000000)
+#define COND_BITS(i)	(i & 0xf0000000)

 #define LDST_I_BIT(i)	(i & (1 << 26))		/* Immediate constant	*/
 #define LDST_P_BIT(i)	(i & (1 << 24))		/* Preindex		*/
@@ -817,6 +818,8 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		break;

 	case 0x04000000:	/* ldr or str immediate */
+		if (COND_BITS(instr) == 0xf0000000) /* NEON VLDn, VSTn */
+			goto bad;
 		offset.un = OFFSET_BITS(instr);
 		handler = do_alignment_ldrstr;
 		break;
--
1.7.9.5

[PATCH 0/2] arm: alignment trap tweaks

[Robin Murphy]

On 07/05/14 10:51, Robin Murphy wrote:
> Hi all,
>
> Revisiting the alignment trap again thanks to an obscure corner case
> with NEON alignment hints - patch 1 is a repost from a while back as a
> ping, because modern userspaces really should just get a resounding
> "Don't do that!" if they manage to step outside the architecture.
> Patch 2 is the fix for cases where fixup does need to be turned back on
> for whatever reason.
>

Ping. Any comment on these? Admittedly it's a very hard-to-hit bug, but 
it's a bug all the same.

Thanks,
Robin.

> The following testcase compiled with -marm illustrates the problem:
> with fixup enabled, when the VLD1 instruction with the alignment hint
> set faults by performing a misaligned access, it fails to load the NEON
> registers as expected and eventually writes back nonsense to the base
> register, resulting in a misleading segfault on the next iteration.
>
> --->8---
>
> #include <stdint.h>
> #include <stdio.h>
>
> static uint8_t buffer[256], got[256];
>
> int main() {
> 	uint8_t *base = (uint8_t *)((intptr_t)buffer + 0x7 & ~0x7);
> 	void *p = base, *q = got;
> 	int i;
>
> 	for (i=0; i<64; i++)
> 		base[i] = i;
> 	for (i=0; i<4; i++) {
> 		asm volatile ("vld1.8 {d0,d1}, [%0]\n vst1.8 {d0,d1}, [%1]\n"
> 				: "=r"(p),"=r"(q) : "0"(p),"1"(q) : "d0","d1");
> 		printf("unaligned: %p [%d,%d,%d,...]\n", p++, got[0], got[1], got[2]);
> 	}
> 	p = base;
> 	for (i=0; i<4; i++) {
> 		asm volatile ("vld1.8 {d0,d1}, [%0:64]\n vst1.8 {d0,d1}, [%1]\n"
> 				: "=r"(p),"=r"(q) : "0"(p),"1"(q) : "d0","d1");
> 		printf("misaligned: %p [%d,%d,%d,...]\n", p++, got[0], got[1], got[2]);
> 	}
> 	return 0;
> }
>
> --->8---
>
> Robin Murphy (2):
>    arm: SIGBUS on unsupported ARMv6 unaligned accesses
>    arm: don't break misaligned NEON load/store
>
>   arch/arm/mm/alignment.c |    9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> --
> 1.7.9.5
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>