From 8c5683006d9aef21eb2875e33f7e54755a4fb681 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Wed, 21 Mar 2018 22:01:17 +0100 Subject: [PATCH 1/5] kernel/arm: do not treat all data aborts as pagefaults For now, distinguish alignment, translation and permission faults. The first kind of faults cause the kernel to send SIGBUS to the process causing the fault, the latter two are forwarded to `vm' as pagefaults. Previously, any data abort was forwarded to `vm' as a pagefault, resulting in hard to debug issue #104. Any unhandled fault status results in a disaster. This seems better than naively hoping `vm' can do something about it. --- minix/kernel/arch/earm/exception.c | 30 +++++++++++++++++++++- minix/kernel/arch/earm/include/archconst.h | 19 ++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/minix/kernel/arch/earm/exception.c b/minix/kernel/arch/earm/exception.c index 19c400822..1930b24ac 100644 --- a/minix/kernel/arch/earm/exception.c +++ b/minix/kernel/arch/earm/exception.c @@ -109,6 +109,34 @@ static void pagefault( struct proc *pr, return; } +static void +data_abort(int is_nested, struct proc *pr, reg_t *saved_lr, + struct ex_s *ep, u32_t dfar, u32_t dfsr) +{ + /* Extract fault status bit [0:3, 10] from DFSR */ + u32_t fs = dfsr & 0x0F; + fs |= ((dfsr >> 6) & 0x10); + if (is_alignment_fault(fs)) { + if (is_nested) { + printf("KERNEL: alignment fault dfar=0x%lx\n", dfar); + inkernel_disaster(pr, saved_lr, ep, is_nested); + } + /* Send SIGBUS to violating process. */ + cause_sig(proc_nr(pr), SIGBUS); + return; + } else if (is_translation_fault(fs) || is_permission_fault(fs)) { + /* Ask VM to handle translation and permission faults as pagefaults */ + pagefault(pr, saved_lr, is_nested, dfar, dfsr); + return; + } else { + /* Die on unknown things... */ + printf("KERNEL: unhandled data abort dfar=0x%lx dfsr=0x%lx " + "fs=0x%lx is_nested=%d\n", dfar, dfsr, fs, is_nested); + panic("unhandled data abort"); + } + NOT_REACHABLE; +} + static void inkernel_disaster(struct proc *saved_proc, reg_t *saved_lr, struct ex_s *ep, int is_nested) @@ -171,7 +199,7 @@ void exception_handler(int is_nested, reg_t *saved_lr, int vector) } if (vector == DATA_ABORT_VECTOR) { - pagefault(saved_proc, saved_lr, is_nested, read_dfar(), read_dfsr()); + data_abort(is_nested, saved_proc, saved_lr, ep, read_dfar(), read_dfsr()); return; } diff --git a/minix/kernel/arch/earm/include/archconst.h b/minix/kernel/arch/earm/include/archconst.h index 8d2edf967..b52fe6a59 100644 --- a/minix/kernel/arch/earm/include/archconst.h +++ b/minix/kernel/arch/earm/include/archconst.h @@ -21,6 +21,25 @@ #define INTERRUPT_VECTOR 6 #define FAST_INTERRUPT_VECTOR 7 + +/* Known fault status bits */ +#define DFSR_FS_ALIGNMENT_FAULT 0x01 +#define DFSR_FS_TRANSLATION_FAULT_PAGE 0x07 +#define DFSR_FS_TRANSLATION_FAULT_SECTION 0x05 +#define DFSR_FS_PERMISSION_FAULT_PAGE 0x0F +#define DFSR_FS_PERMISSION_FAULT_SECTION 0x0D + +#define is_alignment_fault(fault_status) \ + ((fault_status) == DFSR_FS_ALIGNMENT_FAULT) + +#define is_translation_fault(fault_status) \ + (((fault_status) == DFSR_FS_TRANSLATION_FAULT_PAGE) \ + || ((fault_status) == DFSR_FS_TRANSLATION_FAULT_SECTION)) + +#define is_permission_fault(fault_status) \ + (((fault_status) == DFSR_FS_PERMISSION_FAULT_PAGE) \ + || ((fault_status) == DFSR_FS_PERMISSION_FAULT_SECTION)) + /* * defines how many bytes are reserved at the top of the kernel stack for global * information like currently scheduled process or current cpu id From 3b5ef1d60f74e053f4841663ae08d6845c2b598b Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Wed, 21 Mar 2018 20:29:58 +0100 Subject: [PATCH 2/5] bsd.own.mk: use -mno-unaligned-access on ARM Without this option, gcc may emit code accessing unaligned memory. This, and the fact that SCTRL.A (System Control Register - Alignment Check) is set to 1 in Minix causes data aborts when such code is encountered. This was the cause of #104. The `minix-service' executable caused unaligned memory accesses calling into getpwnam(). These then trigger data abort exceptions. On ARM, these were previously forwarded to `vm' as pagefaults. However, `vm' did not properly handle them, but instead allocated one page for the faulting address (over and over again) and then resumed the process at the faulting instruction (over and over again). This behavior masked the whole story as an OOM. Below the assembly version getpwent.c in which unaligned memory accesses are even highlighted... ... 341 ldr lr, [sp, #48] 342 cmp lr, #0 343 bne .L46 344 ldr r0, [r4] @ unaligned 345 add r1, r7, #5 346 str r0, [sp, #4] @ unaligned 347 ldr r4, [sp, #4] 348 mov r5, r4, asr #31 349 strd r4, [r8, #40] ... This should fix #104. It was tested on an actual Beaglebone Black. An alternative fix would be to disable alignment checking by setting SCTRL.A to 0 and allowing unaligned memory accesses. --- share/mk/bsd.own.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/share/mk/bsd.own.mk b/share/mk/bsd.own.mk index f76d5f8a7..946e5dece 100644 --- a/share/mk/bsd.own.mk +++ b/share/mk/bsd.own.mk @@ -82,6 +82,11 @@ SMP_FLAGS += -DCONFIG_MAX_CPUS=${CONFIG_MAX_CPUS} CPPFLAGS+= ${SMP_FLAGS} +# Disabled unaligned accesses on ARM +.if !empty(MACHINE_ARCH:Mearm*) +CFLAGS+= -mno-unaligned-access +.endif + __uname_s!= uname -s .if ${__uname_s:Uunknown} == "Minix" USETOOLS?= never From 612cc57722a71daee07393982af260b7854a75ff Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 22 Mar 2018 14:27:32 +0100 Subject: [PATCH 3/5] arm/archconst: use values defined in armreg.h --- minix/kernel/arch/earm/include/archconst.h | 23 +++++++--------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/minix/kernel/arch/earm/include/archconst.h b/minix/kernel/arch/earm/include/archconst.h index b52fe6a59..d85dd81e0 100644 --- a/minix/kernel/arch/earm/include/archconst.h +++ b/minix/kernel/arch/earm/include/archconst.h @@ -21,24 +21,15 @@ #define INTERRUPT_VECTOR 6 #define FAST_INTERRUPT_VECTOR 7 +/* Data abort helper */ +#define is_align_fault(fault_status) \ + ((fault_status) == FAULT_ALIGN_0) -/* Known fault status bits */ -#define DFSR_FS_ALIGNMENT_FAULT 0x01 -#define DFSR_FS_TRANSLATION_FAULT_PAGE 0x07 -#define DFSR_FS_TRANSLATION_FAULT_SECTION 0x05 -#define DFSR_FS_PERMISSION_FAULT_PAGE 0x0F -#define DFSR_FS_PERMISSION_FAULT_SECTION 0x0D +#define is_trans_fault(fault_status) \ + (((fault_status) == FAULT_TRANS_S) || ((fault_status) == FAULT_TRANS_P)) -#define is_alignment_fault(fault_status) \ - ((fault_status) == DFSR_FS_ALIGNMENT_FAULT) - -#define is_translation_fault(fault_status) \ - (((fault_status) == DFSR_FS_TRANSLATION_FAULT_PAGE) \ - || ((fault_status) == DFSR_FS_TRANSLATION_FAULT_SECTION)) - -#define is_permission_fault(fault_status) \ - (((fault_status) == DFSR_FS_PERMISSION_FAULT_PAGE) \ - || ((fault_status) == DFSR_FS_PERMISSION_FAULT_SECTION)) +#define is_perm_fault(fault_status) \ + (((fault_status) == FAULT_PERM_S) || ((fault_status) == FAULT_PERM_P)) /* * defines how many bytes are reserved at the top of the kernel stack for global From da235143339b1ab7200e085679db53b35e4c1452 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 22 Mar 2018 14:59:20 +0100 Subject: [PATCH 4/5] kernel/arm: send SIGSEGV to processes causing unknown data aborts On second thought, handle unknown faults caused by processes by sending SIGSEGV to them instead of bringing the whole system to a grind. --- minix/kernel/arch/earm/exception.c | 36 ++++++++++++++++-------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/minix/kernel/arch/earm/exception.c b/minix/kernel/arch/earm/exception.c index 1930b24ac..3a358d79d 100644 --- a/minix/kernel/arch/earm/exception.c +++ b/minix/kernel/arch/earm/exception.c @@ -116,25 +116,27 @@ data_abort(int is_nested, struct proc *pr, reg_t *saved_lr, /* Extract fault status bit [0:3, 10] from DFSR */ u32_t fs = dfsr & 0x0F; fs |= ((dfsr >> 6) & 0x10); - if (is_alignment_fault(fs)) { - if (is_nested) { - printf("KERNEL: alignment fault dfar=0x%lx\n", dfar); - inkernel_disaster(pr, saved_lr, ep, is_nested); - } - /* Send SIGBUS to violating process. */ - cause_sig(proc_nr(pr), SIGBUS); - return; - } else if (is_translation_fault(fs) || is_permission_fault(fs)) { - /* Ask VM to handle translation and permission faults as pagefaults */ + + /* Translation and permission faults are handled as pagefaults. */ + if (is_trans_fault(fs) || is_perm_fault(fs)) { pagefault(pr, saved_lr, is_nested, dfar, dfsr); - return; - } else { - /* Die on unknown things... */ - printf("KERNEL: unhandled data abort dfar=0x%lx dfsr=0x%lx " - "fs=0x%lx is_nested=%d\n", dfar, dfsr, fs, is_nested); - panic("unhandled data abort"); + } else if (!is_nested) { + /* A user process caused some other kind of data abort. */ + int signum = SIGSEGV; + + if (is_align_fault(fs)) { + signum = SIGBUS; + } else { + printf("KERNEL: unknown data abort by proc %d sending " + "SIGSEGV (dfar=0x%lx dfsr=0x%lx fs=0x%lx)\n", + proc_nr(pr), dfar, dfsr, fs); + } + cause_sig(proc_nr(pr), signum); + } else { /* is_nested */ + printf("KERNEL: inkernel data abort - disaster (dfar=0x%lx " + "dfsr=0x%lx fs=0x%lx)\n", dfar, dfsr, fs); + inkernel_disaster(pr, saved_lr, ep, is_nested); } - NOT_REACHABLE; } static void inkernel_disaster(struct proc *saved_proc, From 3b71b6492da82b304d982e220962370579bac319 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 22 Mar 2018 15:47:36 +0100 Subject: [PATCH 5/5] minix/tests/arm: naive tests to cause data aborts Some assembly code to cause unaligned access as well as segmentation faults to exercise the data abort path. --- minix/tests/Makefile | 2 ++ minix/tests/arch/earm/Makefile.inc | 7 ++++++ minix/tests/arch/earm/test_arm_segfault.S | 16 +++++++++++++ minix/tests/arch/earm/test_arm_unaligned.S | 26 ++++++++++++++++++++++ minix/tests/arch/i386/Makefile.inc | 0 5 files changed, 51 insertions(+) create mode 100644 minix/tests/arch/earm/Makefile.inc create mode 100644 minix/tests/arch/earm/test_arm_segfault.S create mode 100644 minix/tests/arch/earm/test_arm_unaligned.S create mode 100644 minix/tests/arch/i386/Makefile.inc diff --git a/minix/tests/Makefile b/minix/tests/Makefile index c0bcc8557..4662fe872 100644 --- a/minix/tests/Makefile +++ b/minix/tests/Makefile @@ -126,6 +126,8 @@ PROGS+= test63 mod OBJS.${o} += common.o .endfor +.include "./arch/${MACHINE_ARCH}/Makefile.inc" + # LSC Make sure there is not leftover after a failed testrun clean: .PHONY .MAKE @rm -rf DIR* diff --git a/minix/tests/arch/earm/Makefile.inc b/minix/tests/arch/earm/Makefile.inc new file mode 100644 index 000000000..18c590c9b --- /dev/null +++ b/minix/tests/arch/earm/Makefile.inc @@ -0,0 +1,7 @@ +PROGS+= test_arm_segfault +PROGS+= test_arm_unaligned + +.PATH: ${.CURDIR}/arch/${MACHINE_ARCH} + +test_arm_segfault.o : test_arm_segfault.S +test_arm_unaligned.o : test_arm_unaligned.S diff --git a/minix/tests/arch/earm/test_arm_segfault.S b/minix/tests/arch/earm/test_arm_segfault.S new file mode 100644 index 000000000..db13dc544 --- /dev/null +++ b/minix/tests/arch/earm/test_arm_segfault.S @@ -0,0 +1,16 @@ +.text +.global main +main: + push {lr} + ldr r0, =0xDEADBEE0 /* Hopefully this is not mapped... */ + ldr r1, [r0] + ldr r0, =0x01010100 /* In case we survived, try something else */ + ldr r1, [r0] + + ldr r0, =msg + bl puts + + mov r0, #0 /* test should check for non-zero exit code / signal */ + pop {pc} +msg: + .ascii "ERROR - caused no segfault\n" diff --git a/minix/tests/arch/earm/test_arm_unaligned.S b/minix/tests/arch/earm/test_arm_unaligned.S new file mode 100644 index 000000000..1d4486e7f --- /dev/null +++ b/minix/tests/arch/earm/test_arm_unaligned.S @@ -0,0 +1,26 @@ +.text +.global main +main: + push {lr} + mov r0, sp + + /* This should work */ + ldr r0, [sp] + + /* Unalign it */ + add r0, #2 + + /* Try a non-word aligned word-load, this may work if SCTRL.A == 0 */ + ldr r1, [r0] + + /* Load non-word aligned dword, should die even with SCTRL.A == 0 */ + ldrd r2, r3, [r0] + + + ldr r0, =msg + bl puts + + mov r0, #0 /* test should check for non-zero exit code / signal */ + pop {pc} +msg: + .ascii "ERROR - caused no sigbus\n" diff --git a/minix/tests/arch/i386/Makefile.inc b/minix/tests/arch/i386/Makefile.inc new file mode 100644 index 000000000..e69de29bb