Message ID | 1311689686-7451-1-git-send-email-dave.martin@linaro.org |
---|---|
State | Superseded |
Headers | show |
Note that there is no PATCH 2/2 -- I had a mishap with git format-patch, dumping out more patches than I intended to... Cheers ---Dave
On Tue, Jul 26, 2011 at 03:14:46PM +0100, Dave Martin wrote: > With the UM_SIGNAL alignment fault mode, no siginfo structure is > passed to userspace. > > POSIX specifies how siginfo_t should be populated for alignment > faults, so this patch does just that: > > * si_signo = SIGBUS > * si_code = BUS_ADRALN > * si_addr = address of the faulted instruction > > Signed-off-by: Dave Martin <dave.martin@linaro.org> > --- > arch/arm/mm/alignment.c | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c > index 724ba3b..9a65aaf 100644 > --- a/arch/arm/mm/alignment.c > +++ b/arch/arm/mm/alignment.c > @@ -22,6 +22,7 @@ > #include <linux/sched.h> > #include <linux/uaccess.h> > > +#include <asm/system.h> > #include <asm/unaligned.h> > > #include "fault.h" > @@ -883,9 +884,16 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) > if (ai_usermode & UM_FIXUP) > goto fixup; > > - if (ai_usermode & UM_SIGNAL) > - force_sig(SIGBUS, current); > - else { > + if (ai_usermode & UM_SIGNAL) { > + siginfo_t si; > + > + si.si_signo = SIGBUS; > + si.si_errno = 0; > + si.si_code = BUS_ADRALN; > + si.si_addr = (void __user *)instruction_pointer(regs); This is wrong. You need something like: si.si_addr = (void __user *)instruction_pointer(regs) - (thumb_mode(regs) ? 2 : 4); > + > + force_sig_info(si.si_signo, &si, current); > + } else { > /* > * We're about to disable the alignment trap and return to > * user space. But if an interrupt occurs before actually > -- > 1.7.4.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Jul 26, 2011 at 07:29:05PM +0300, Kirill A. Shutemov wrote: > On Tue, Jul 26, 2011 at 03:14:46PM +0100, Dave Martin wrote: > > With the UM_SIGNAL alignment fault mode, no siginfo structure is > > passed to userspace. > > > > POSIX specifies how siginfo_t should be populated for alignment > > faults, so this patch does just that: > > > > * si_signo = SIGBUS > > * si_code = BUS_ADRALN > > * si_addr = address of the faulted instruction > > > > Signed-off-by: Dave Martin <dave.martin@linaro.org> > > --- > > arch/arm/mm/alignment.c | 14 +++++++++++--- > > 1 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c [...] > > @@ -883,9 +884,16 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) [...] > > + if (ai_usermode & UM_SIGNAL) { > > + siginfo_t si; > > + > > + si.si_signo = SIGBUS; > > + si.si_errno = 0; > > + si.si_code = BUS_ADRALN; > > + si.si_addr = (void __user *)instruction_pointer(regs); > > This is wrong. You need something like: > > si.si_addr = (void __user *)instruction_pointer(regs) - > (thumb_mode(regs) ? 2 : 4); I don't think so. The appropriate adjustment is already made on vector entry by the vector_stub macro in entry-armv.S: .macro vector_stub, name, mode, correction=0 .align 5 vector_\name: .if \correction sub lr, lr, #\correction .endif I'm pretty sure that instruction_pointer(regs) must already point to the faulted instruction when we enter do_alignment(), because the first thing this function does is: instrptr = instruction_pointer(regs); if (thumb_mode(regs)) { fault = __get_user(tinstr, (u16 *)(instrptr & ~1)); /* ... */ } else fault = __get_user(instr, (u32 *)instrptr); When I test the code, my observations bear this out: the address returned in si_addr does match the address of the faulting instruction. Does that satisfy your concerns, or have I missed something? It might make sense to set bit 1 of si_addr to match the Thumb-ness of the faulting instruction though. Currently I don't do that. Cheers ---Dave
On Tue, Jul 26, 2011 at 06:16:42PM +0100, Dave Martin wrote:
> Does that satisfy your concerns, or have I missed something?
You're right. Sorry.
On Tue, Jul 26, 2011 at 03:14:46PM +0100, Dave Martin wrote: > With the UM_SIGNAL alignment fault mode, no siginfo structure is > passed to userspace. > > POSIX specifies how siginfo_t should be populated for alignment > faults, so this patch does just that: > > * si_signo = SIGBUS > * si_code = BUS_ADRALN > * si_addr = address of the faulted instruction > > Signed-off-by: Dave Martin <dave.martin@linaro.org> Reviewed-by: Kirill A. Shutemov <kirill@shutemov.name> > --- > arch/arm/mm/alignment.c | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c > index 724ba3b..9a65aaf 100644 > --- a/arch/arm/mm/alignment.c > +++ b/arch/arm/mm/alignment.c > @@ -22,6 +22,7 @@ > #include <linux/sched.h> > #include <linux/uaccess.h> > > +#include <asm/system.h> > #include <asm/unaligned.h> > > #include "fault.h" > @@ -883,9 +884,16 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) > if (ai_usermode & UM_FIXUP) > goto fixup; > > - if (ai_usermode & UM_SIGNAL) > - force_sig(SIGBUS, current); > - else { > + if (ai_usermode & UM_SIGNAL) { > + siginfo_t si; > + > + si.si_signo = SIGBUS; > + si.si_errno = 0; > + si.si_code = BUS_ADRALN; > + si.si_addr = (void __user *)instruction_pointer(regs); > + > + force_sig_info(si.si_signo, &si, current); > + } else { > /* > * We're about to disable the alignment trap and return to > * user space. But if an interrupt occurs before actually > -- > 1.7.4.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, 26 Jul 2011, Dave Martin wrote: > With the UM_SIGNAL alignment fault mode, no siginfo structure is > passed to userspace. > > POSIX specifies how siginfo_t should be populated for alignment > faults, so this patch does just that: > > * si_signo = SIGBUS > * si_code = BUS_ADRALN > * si_addr = address of the faulted instruction > > Signed-off-by: Dave Martin <dave.martin@linaro.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> > --- > arch/arm/mm/alignment.c | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c > index 724ba3b..9a65aaf 100644 > --- a/arch/arm/mm/alignment.c > +++ b/arch/arm/mm/alignment.c > @@ -22,6 +22,7 @@ > #include <linux/sched.h> > #include <linux/uaccess.h> > > +#include <asm/system.h> > #include <asm/unaligned.h> > > #include "fault.h" > @@ -883,9 +884,16 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) > if (ai_usermode & UM_FIXUP) > goto fixup; > > - if (ai_usermode & UM_SIGNAL) > - force_sig(SIGBUS, current); > - else { > + if (ai_usermode & UM_SIGNAL) { > + siginfo_t si; > + > + si.si_signo = SIGBUS; > + si.si_errno = 0; > + si.si_code = BUS_ADRALN; > + si.si_addr = (void __user *)instruction_pointer(regs); > + > + force_sig_info(si.si_signo, &si, current); > + } else { > /* > * We're about to disable the alignment trap and return to > * user space. But if an interrupt occurs before actually > -- > 1.7.4.1 >
diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c index 724ba3b..9a65aaf 100644 --- a/arch/arm/mm/alignment.c +++ b/arch/arm/mm/alignment.c @@ -22,6 +22,7 @@ #include <linux/sched.h> #include <linux/uaccess.h> +#include <asm/system.h> #include <asm/unaligned.h> #include "fault.h" @@ -883,9 +884,16 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) if (ai_usermode & UM_FIXUP) goto fixup; - if (ai_usermode & UM_SIGNAL) - force_sig(SIGBUS, current); - else { + if (ai_usermode & UM_SIGNAL) { + siginfo_t si; + + si.si_signo = SIGBUS; + si.si_errno = 0; + si.si_code = BUS_ADRALN; + si.si_addr = (void __user *)instruction_pointer(regs); + + force_sig_info(si.si_signo, &si, current); + } else { /* * We're about to disable the alignment trap and return to * user space. But if an interrupt occurs before actually
With the UM_SIGNAL alignment fault mode, no siginfo structure is passed to userspace. POSIX specifies how siginfo_t should be populated for alignment faults, so this patch does just that: * si_signo = SIGBUS * si_code = BUS_ADRALN * si_addr = address of the faulted instruction Signed-off-by: Dave Martin <dave.martin@linaro.org> --- arch/arm/mm/alignment.c | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-)