* [PATCH] fix armv8 kernel generation of SIGSEGV upon unaligned access @ 2017-04-03 5:45 Victor Kamensky 2017-04-03 5:45 ` [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS Victor Kamensky 0 siblings, 1 reply; 5+ messages in thread From: Victor Kamensky @ 2017-04-03 5:45 UTC (permalink / raw) To: linux-arm-kernel Hi Will, Catalin, I discovered that on latest ARMv8 kernel, starting with 4.5, unaligned access by aarch32 ldm instruction (and similar others) produces SIGSEGV to process, which seems to be wrong. On 4.4 kernel the same test produced SIGBUS. The issue was tracked back to 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses) Following patch proposes the fix. And here are more details and test case that illustrates the issue. Please note I do not have an idea how to reproduce the issue mentioned by 52d7523. I presume that __do_kernel_fault called by do_bad_area will still take care of it. Tested system: armv8 kernel and aarch32 rootfs ---------------------------------------------- Note it is ARMv8 kernel and aarch32 rootfs. I thought using unaligned aarch32 ldm is good way to illustrate the issue. My understanding that with aarch64 instructions only unaligned access to device memory will produce unaligned exception, but which is much harder to reproduce. root at arm-sdk-generic:~/align# uname -a Linux arm-sdk-generic 4.4.58 #1 SMP PREEMPT Thu Mar 30 21:03:34 PDT 2017 aarch64 aarch64 aarch64 GNU/Linux root at arm-sdk-generic:~/align# file /lib/systemd/systemd /lib/systemd/systemd: ELF 32-bit LSB shared object, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 2.6.32, BuildID[sha1]=5c8e03b21f46227d69ad8690ed80ba4ff54b61b3, stripped Test Code: that produces unaligned access with ldm instructions --------------------------------------------------------------- root at arm-sdk-generic:~/align# ls Makefile align.h align1.c main.c root at arm-sdk-generic:~/align# cat main.c #include <stdio.h> #include <signal.h> #include <unistd.h> #include "align.h" static void sigbus_sa_sigaction(int sig, siginfo_t *context, void *uctx) { printf("%s: got sigbus, sig = %d\n", __FUNCTION__, sig); _exit(1); } static void sigsegv_sa_sigaction(int sig, siginfo_t *context, void *uctx) { printf("%s: got sigsegv, should not be here, sig = %d\n", __FUNCTION__, sig); _exit(2); } void register_signals(void) { struct sigaction sa, sa_old; sa.sa_sigaction = sigbus_sa_sigaction; sa.sa_flags = SA_SIGINFO; sigaction(SIGBUS, &sa, &sa_old); sa.sa_sigaction = sigsegv_sa_sigaction; sigaction(SIGSEGV, &sa, &sa_old); } int main (void) { char buf[]="0123456789012345"; int result; register_signals(); result = load_two((struct twoelem *) (buf + 1)); printf("result = 0x%x\n", result); return 0; } root at arm-sdk-generic:~/align# cat align.h struct twoelem { int i; int j; }; int load_two(struct twoelem *); root at arm-sdk-generic:~/align# cat align1.c #include "align.h" int load_two (struct twoelem *s) { return s->i + s->j; } root at arm-sdk-generic:~/align# cat Makefile all: align CC=gcc CFLAGS=-O2 -g align: align1.o main.o $(CC) $(CFLAGS) -o $@ $^ clean: @rm -r align *.o root at arm-sdk-generic:~/align# make gcc -O2 -g -c -o align1.o align1.c gcc -O2 -g -c -o main.o main.c gcc -O2 -g -o align align1.o main.o root at arm-sdk-generic:~/align# objdump --disassemble align1.o align1.o: file format elf32-littlearm Disassembly of section .text: 00000000 <load_two>: 0: e8900005 ldm r0, {r0, r2} 4: e0820000 add r0, r2, r0 8: e12fff1e bx lr root at arm-sdk-generic:~/align# On 4.4 kernel SIGBUS received and unsolicted message from kernel produced ------------------------------------------------------------------------- That is what we see on our 4.4 kernel: correct signal SIGBUS is invoked. But kernel produces unsolicited message, "Unhandled fault: alignment fault", for every signal: that is what I wanted to fix first. root at arm-sdk-generic:~/align# uname -a Linux arm-sdk-generic 4.4.58 #1 SMP PREEMPT Thu Mar 30 21:03:34 PDT 2017 aarch64 aarch64 aarch64 GNU/Linux root at arm-sdk-generic:~/align# ./align Unhandled fault: alignment fault (0x92000021) at 0x00000000fff6b955 sigbus_sa_sigaction: got sigbus, sig = 7 root at arm-sdk-generic:~/align# On 4.11-rc4 kernel SIGSEGV received upon the same fault ------------------------------------------------------- On latest kernel when I've tried the same test I discovered that process now gets SIGSEGV instead of SIGBUS when unaligned ldm instruction is executed. Tracked it down to 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses), which got at kernel starting with 4.5. root at arm-sdk-generic:~/align# uname -a Linux arm-sdk-generic 4.11.0-rc4 #1 SMP PREEMPT Thu Mar 30 20:19:56 PDT 2017 aarch64 aarch64 aarch64 GNU/Linux root at arm-sdk-generic:~/align# ./align sigsegv_sa_sigaction: got sigsegv, should not be here, sig = 11 root at arm-sdk-generic:~/align# On 4.11-rc4 with my fix, back to SIGBUS --------------------------------------- After my proposed fix, getting SIGBUS again. root at arm-sdk-generic:~/align# uname -a Linux arm-sdk-generic 4.11.0-rc4+ #2 SMP PREEMPT Thu Mar 30 23:53:36 PDT 2017 aarch64 aarch64 aarch64 GNU/Linux root at arm-sdk-generic:~/align# ./align sigbus_sa_sigaction: got sigbus, sig = 7 root at arm-sdk-generic:~/align# Thanks, Victor Victor Kamensky (1): arm64: mm: unaligned access by user-land should be received as SIGBUS arch/arm64/mm/fault.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS 2017-04-03 5:45 [PATCH] fix armv8 kernel generation of SIGSEGV upon unaligned access Victor Kamensky @ 2017-04-03 5:45 ` Victor Kamensky 2017-04-03 9:24 ` Will Deacon 0 siblings, 1 reply; 5+ messages in thread From: Victor Kamensky @ 2017-04-03 5:45 UTC (permalink / raw) To: linux-arm-kernel After 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses) commit user-land accesses that produce unaligned exceptions like in case of aarch32 ldm/stm/ldrd/strd instructions operating on unaligned memory received by user-land as SIGSEGV. It is wrong, it should be reported as SIGBUS as it was before 52d7523 commit. Changed do_bad_area function to take signal and code parameters, so caller can pass them down properly depending on fault type, as SIGSEGV in case of do_translation_fault and SIGBUS in case of do_alignment_fault. Signed-off-by: Victor Kamensky <kamensky@cisco.com> Cc: xe-linux-external at cisco.com Fixes: 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses) --- arch/arm64/mm/fault.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index 4bf899f..204eb58 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -215,7 +215,8 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr, force_sig_info(sig, &si, tsk); } -static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs) +static void do_bad_area(unsigned long addr, unsigned int esr, + struct pt_regs *regs, int sig, int code) { struct task_struct *tsk = current; struct mm_struct *mm = tsk->active_mm; @@ -225,7 +226,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re * handle this fault with. */ if (user_mode(regs)) - __do_user_fault(tsk, addr, esr, SIGSEGV, SEGV_MAPERR, regs); + __do_user_fault(tsk, addr, esr, sig, code, regs); else __do_kernel_fault(mm, addr, esr, regs); } @@ -469,14 +470,14 @@ static int __kprobes do_translation_fault(unsigned long addr, if (addr < TASK_SIZE) return do_page_fault(addr, esr, regs); - do_bad_area(addr, esr, regs); + do_bad_area(addr, esr, regs, SIGSEGV, SEGV_MAPERR); return 0; } static int do_alignment_fault(unsigned long addr, unsigned int esr, struct pt_regs *regs) { - do_bad_area(addr, esr, regs); + do_bad_area(addr, esr, regs, SIGBUS, BUS_ADRALN); return 0; } -- 1.9.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS 2017-04-03 5:45 ` [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS Victor Kamensky @ 2017-04-03 9:24 ` Will Deacon 2017-04-06 6:01 ` Victor Kamensky 0 siblings, 1 reply; 5+ messages in thread From: Will Deacon @ 2017-04-03 9:24 UTC (permalink / raw) To: linux-arm-kernel Hi Victor, Thanks for reporting this. On Sun, Apr 02, 2017 at 10:45:14PM -0700, Victor Kamensky wrote: > After 52d7523 (arm64: mm: allow the kernel to handle alignment faults on > user accesses) commit user-land accesses that produce unaligned exceptions > like in case of aarch32 ldm/stm/ldrd/strd instructions operating on > unaligned memory received by user-land as SIGSEGV. It is wrong, it should > be reported as SIGBUS as it was before 52d7523 commit. > > Changed do_bad_area function to take signal and code parameters, so caller > can pass them down properly depending on fault type, as SIGSEGV in case of > do_translation_fault and SIGBUS in case of do_alignment_fault. > > Signed-off-by: Victor Kamensky <kamensky@cisco.com> > Cc: xe-linux-external at cisco.com > Fixes: 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses) > --- > arch/arm64/mm/fault.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index 4bf899f..204eb58 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -215,7 +215,8 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr, > force_sig_info(sig, &si, tsk); > } > > -static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs) > +static void do_bad_area(unsigned long addr, unsigned int esr, > + struct pt_regs *regs, int sig, int code) > { > struct task_struct *tsk = current; > struct mm_struct *mm = tsk->active_mm; > @@ -225,7 +226,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re > * handle this fault with. > */ > if (user_mode(regs)) > - __do_user_fault(tsk, addr, esr, SIGSEGV, SEGV_MAPERR, regs); > + __do_user_fault(tsk, addr, esr, sig, code, regs); > else > __do_kernel_fault(mm, addr, esr, regs); > } > @@ -469,14 +470,14 @@ static int __kprobes do_translation_fault(unsigned long addr, > if (addr < TASK_SIZE) > return do_page_fault(addr, esr, regs); > > - do_bad_area(addr, esr, regs); > + do_bad_area(addr, esr, regs, SIGSEGV, SEGV_MAPERR); > return 0; > } > > static int do_alignment_fault(unsigned long addr, unsigned int esr, > struct pt_regs *regs) > { > - do_bad_area(addr, esr, regs); > + do_bad_area(addr, esr, regs, SIGBUS, BUS_ADRALN); Can we not just extract the signal number and code from the fault info table? E.g. leave the type signature of do_bad_area like it is, but do: const struct fault_info *inf = fault_info + (esr & 63); __do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs); The '& 63' is ugly as hell, so maybe wrap that up in a esr_to_fault_info function and kill the fault_name thing we have at the moment. Will ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS 2017-04-03 9:24 ` Will Deacon @ 2017-04-06 6:01 ` Victor Kamensky 2017-04-06 8:44 ` Will Deacon 0 siblings, 1 reply; 5+ messages in thread From: Victor Kamensky @ 2017-04-06 6:01 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On Mon, 3 Apr 2017, Will Deacon wrote: > Hi Victor, > > Thanks for reporting this. > > On Sun, Apr 02, 2017 at 10:45:14PM -0700, Victor Kamensky wrote: >> After 52d7523 (arm64: mm: allow the kernel to handle alignment faults on >> user accesses) commit user-land accesses that produce unaligned exceptions >> like in case of aarch32 ldm/stm/ldrd/strd instructions operating on >> unaligned memory received by user-land as SIGSEGV. It is wrong, it should >> be reported as SIGBUS as it was before 52d7523 commit. >> >> Changed do_bad_area function to take signal and code parameters, so caller >> can pass them down properly depending on fault type, as SIGSEGV in case of >> do_translation_fault and SIGBUS in case of do_alignment_fault. >> >> Signed-off-by: Victor Kamensky <kamensky@cisco.com> >> Cc: xe-linux-external at cisco.com >> Fixes: 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses) >> --- >> arch/arm64/mm/fault.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c >> index 4bf899f..204eb58 100644 >> --- a/arch/arm64/mm/fault.c >> +++ b/arch/arm64/mm/fault.c >> @@ -215,7 +215,8 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr, >> force_sig_info(sig, &si, tsk); >> } >> >> -static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs) >> +static void do_bad_area(unsigned long addr, unsigned int esr, >> + struct pt_regs *regs, int sig, int code) >> { >> struct task_struct *tsk = current; >> struct mm_struct *mm = tsk->active_mm; >> @@ -225,7 +226,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re >> * handle this fault with. >> */ >> if (user_mode(regs)) >> - __do_user_fault(tsk, addr, esr, SIGSEGV, SEGV_MAPERR, regs); >> + __do_user_fault(tsk, addr, esr, sig, code, regs); >> else >> __do_kernel_fault(mm, addr, esr, regs); >> } >> @@ -469,14 +470,14 @@ static int __kprobes do_translation_fault(unsigned long addr, >> if (addr < TASK_SIZE) >> return do_page_fault(addr, esr, regs); >> >> - do_bad_area(addr, esr, regs); >> + do_bad_area(addr, esr, regs, SIGSEGV, SEGV_MAPERR); >> return 0; >> } >> >> static int do_alignment_fault(unsigned long addr, unsigned int esr, >> struct pt_regs *regs) >> { >> - do_bad_area(addr, esr, regs); >> + do_bad_area(addr, esr, regs, SIGBUS, BUS_ADRALN); > > Can we not just extract the signal number and code from the fault info > table? > > E.g. leave the type signature of do_bad_area like it is, but do: > > const struct fault_info *inf = fault_info + (esr & 63); > __do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs); > > The '& 63' is ugly as hell, so maybe wrap that up in a esr_to_fault_info > function and kill the fault_name thing we have at the moment. Could you please take a look at [1]. I've tried to reimplement the fix following your suggestion. Initially I wanted to fix do_bad_area function with use of fault_info array as you suggested, but realized that fix would be more involved, since struct fault_info and array fault_info defined together and one would need to move either do_bad_area function below fault_info array definition or move struct fault_info above it. In my [1] fix I choose the latter. [1] http://archive.armlinux.org.uk/lurker/message/20170404.055101.e6d54d8e.en.html Thanks, Victor > Will > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS 2017-04-06 6:01 ` Victor Kamensky @ 2017-04-06 8:44 ` Will Deacon 0 siblings, 0 replies; 5+ messages in thread From: Will Deacon @ 2017-04-06 8:44 UTC (permalink / raw) To: linux-arm-kernel Hi Victor, On Wed, Apr 05, 2017 at 11:01:45PM -0700, Victor Kamensky wrote: > On Mon, 3 Apr 2017, Will Deacon wrote: > >On Sun, Apr 02, 2017 at 10:45:14PM -0700, Victor Kamensky wrote: > >>After 52d7523 (arm64: mm: allow the kernel to handle alignment faults on > >>user accesses) commit user-land accesses that produce unaligned exceptions > >>like in case of aarch32 ldm/stm/ldrd/strd instructions operating on > >>unaligned memory received by user-land as SIGSEGV. It is wrong, it should > >>be reported as SIGBUS as it was before 52d7523 commit. > >> > >>Changed do_bad_area function to take signal and code parameters, so caller > >>can pass them down properly depending on fault type, as SIGSEGV in case of > >>do_translation_fault and SIGBUS in case of do_alignment_fault. > >> > >>Signed-off-by: Victor Kamensky <kamensky@cisco.com> > >>Cc: xe-linux-external at cisco.com > >>Fixes: 52d7523 (arm64: mm: allow the kernel to handle alignment faults on user accesses) > >>--- > >> arch/arm64/mm/fault.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >>diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > >>index 4bf899f..204eb58 100644 > >>--- a/arch/arm64/mm/fault.c > >>+++ b/arch/arm64/mm/fault.c > >>@@ -215,7 +215,8 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr, > >> force_sig_info(sig, &si, tsk); > >> } > >> > >>-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs) > >>+static void do_bad_area(unsigned long addr, unsigned int esr, > >>+ struct pt_regs *regs, int sig, int code) > >> { > >> struct task_struct *tsk = current; > >> struct mm_struct *mm = tsk->active_mm; > >>@@ -225,7 +226,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re > >> * handle this fault with. > >> */ > >> if (user_mode(regs)) > >>- __do_user_fault(tsk, addr, esr, SIGSEGV, SEGV_MAPERR, regs); > >>+ __do_user_fault(tsk, addr, esr, sig, code, regs); > >> else > >> __do_kernel_fault(mm, addr, esr, regs); > >> } > >>@@ -469,14 +470,14 @@ static int __kprobes do_translation_fault(unsigned long addr, > >> if (addr < TASK_SIZE) > >> return do_page_fault(addr, esr, regs); > >> > >>- do_bad_area(addr, esr, regs); > >>+ do_bad_area(addr, esr, regs, SIGSEGV, SEGV_MAPERR); > >> return 0; > >> } > >> > >> static int do_alignment_fault(unsigned long addr, unsigned int esr, > >> struct pt_regs *regs) > >> { > >>- do_bad_area(addr, esr, regs); > >>+ do_bad_area(addr, esr, regs, SIGBUS, BUS_ADRALN); > > > >Can we not just extract the signal number and code from the fault info > >table? > > > >E.g. leave the type signature of do_bad_area like it is, but do: > > > > const struct fault_info *inf = fault_info + (esr & 63); > > __do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs); > > > >The '& 63' is ugly as hell, so maybe wrap that up in a esr_to_fault_info > >function and kill the fault_name thing we have at the moment. > > Could you please take a look at [1]. I've tried to reimplement the > fix following your suggestion. Yup, I picked this up here already: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=fixes/core Thanks, Will ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-06 8:44 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-03 5:45 [PATCH] fix armv8 kernel generation of SIGSEGV upon unaligned access Victor Kamensky 2017-04-03 5:45 ` [PATCH] arm64: mm: unaligned access by user-land should be received as SIGBUS Victor Kamensky 2017-04-03 9:24 ` Will Deacon 2017-04-06 6:01 ` Victor Kamensky 2017-04-06 8:44 ` Will Deacon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.