arm: kprobes: Align stack to 8-bytes in test code

Without this fix, some test cases will generate alignment faults on systems where alignment is enforced. Even if the kernel is configured to handle these faults in software, triggering them is ugly. It also exposes limitations in the fault handling code which doesn't cope with writes to the stack. E.g. when handling this instruction

   strd r6, [sp, #-64]!

the fault handling code will write to a stack location below the SP value at the point the fault occurred, which coincides with where the exception handler has pushed the saved register context. This results in corruption of those registers.

Quoted 6 lines · Jon Medhurst

Without this fix, some test cases will generate alignment faults on systems where alignment is enforced. Even if the kernel is configured to handle these faults in software, triggering them is ugly. It also exposes limitations in the fault handling code which doesn't cope with writes to the stack. E.g. when handling this instruction

Quoted 6 lines

   strd r6, [sp, #-64]!

the fault handling code will write to a stack location below the SP value at the point the fault occurred, which coincides with where the exception handler has pushed the saved register context. This results in corruption of those registers.

The general rule today is that the stack must always be 64-bit aligned, so an even number of registers must always be pushed to the stack.

arch/arm/probes/kprobes/test-core.c static void coverage_end(void) · line 983

Quoted 13 lines

arch/arm/probes/kprobes/test-core.c +4 -1

index c893726aa52d..1c98a87786ca 100644
@@ -977,7 +977,10 @@ static void coverage_end(void)
 void __naked __kprobes_test_case_start(void)
 {
 	__asm__ __volatile__ (
-		"stmdb	sp!, {r4-r11}				\n\t"
+		"mov	r2, sp					\n\t"
+		"bic	r3, r2, #7				\n\t"
+		"mov	sp, r3					\n\t"
+		"stmdb	sp!, {r2-r11}				\n\t"

I'm not sure these is where the problem is - on entry, the stack should be 64-bit aligned

Quoted 17 lines · Russell King - ARM Linux

Quoted 6 lines · Jon Medhurst

Without this fix, some test cases will generate alignment faults on systems where alignment is enforced. Even if the kernel is configured to handle these faults in software, triggering them is ugly. It also exposes limitations in the fault handling code which doesn't cope with writes to the stack. E.g. when handling this instruction

Quoted 6 lines

   strd r6, [sp, #-64]!

the fault handling code will write to a stack location below the SP value at the point the fault occurred, which coincides with where the exception handler has pushed the saved register context. This results in corruption of those registers.

The general rule today is that the stack must always be 64-bit aligned, so an even number of registers must always be pushed to the stack.

Quoted 16 lines

arch/arm/probes/kprobes/test-core.c static void coverage_end(void) · line 983

Quoted 13 lines

arch/arm/probes/kprobes/test-core.c +4 -1

index c893726aa52d..1c98a87786ca 100644
@@ -977,7 +977,10 @@ static void coverage_end(void)
 void __naked __kprobes_test_case_start(void)
 {
 	__asm__ __volatile__ (
-		"stmdb	sp!, {r4-r11}				\n\t"
+		"mov	r2, sp					\n\t"
+		"bic	r3, r2, #7				\n\t"
+		"mov	sp, r3					\n\t"
+		"stmdb	sp!, {r2-r11}				\n\t"

I'm not sure these is where the problem is - on entry, the stack should be 64-bit aligned

It isn't, because GCC turns code like this

void foo(void)
{
asm volatile("bl __kprobes_test_case_start"
: : : "r0", "r1", "r2", "r3", "ip", "lr", "memory", "cc");
}

into this...

8010e4ac <foo>:

8010e4ac:       e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
8010e4b0:       eb002c99        bl      8011971c <__kprobes_test_case_start>
8010e4b4:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

Perhaps we need a way of telling GCC we are using the stack but I've not managed to spot a way of doing that.

Quoted 1 line · Jon Medhurst (Tixy)

Quoted 15 lines

It isn't, because GCC turns code like this

void foo(void)
{
asm volatile("bl __kprobes_test_case_start"
: : : "r0", "r1", "r2", "r3", "ip", "lr", "memory", "cc");
}

into this...

8010e4ac <foo>:

8010e4ac:       e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
8010e4b0:       eb002c99        bl      8011971c <__kprobes_test_case_start>
8010e4b4:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

Quoted 2 lines

Perhaps we need a way of telling GCC we are using the stack but I've not managed to spot a way of doing that.

Using which compiler options?

Quoted 2 lines · Russell King - ARM Linux

Quoted 1 line · Jon Medhurst (Tixy)

[...]

Quoted 15 lines

Quoted 15 lines

It isn't, because GCC turns code like this

void foo(void)
{
asm volatile("bl __kprobes_test_case_start"
: : : "r0", "r1", "r2", "r3", "ip", "lr", "memory", "cc");
}

into this...

8010e4ac <foo>:

8010e4ac:       e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
8010e4b0:       eb002c99        bl      8011971c <__kprobes_test_case_start>
8010e4b4:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

Quoted 4 lines

Quoted 2 lines

Perhaps we need a way of telling GCC we are using the stack but I've not managed to spot a way of doing that.

Using which compiler options?

The ones the Linux makefile picks when building with vexpress_defconfig. I hacked a kernel file to build the above example but the behaviour is what I observed with the real kprobes code. I've pasted the commanline produced by building with V=1 at the end of this email.

One thing I've noticed playing around just now is that if I add "sp" to the clobber list, and use a newer GCC (gcc-linaro-6.2.1-2016.11) then it does allign the stack correctly. "sp" makes no difference with GCC 5.3 or 4.8.

I suspect we're into compiler behavioural differences, which can't be relied upon, so I guess we do need to do something in __kprobes_test_case_start() to work around it.

I'd do:

mov ip, sp
tst sp, #4
subeq sp, sp, #4 @ need to mis-align as we don't save
stmfd sp!, {r4 - r11, ip} @ an even number of registers to end
sub sp, sp, #size @ up with an aligned stack here

and when restoring:

	add	ip, sp, #size
	ldmfd	ip, {r4 - r11, sp}
	bx	r0