* [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct
2011-07-26 14:14 [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct Dave Martin
@ 2011-07-26 14:16 ` Dave Martin
2011-07-26 16:29 ` Kirill A. Shutemov
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Dave Martin @ 2011-07-26 14:16 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct
2011-07-26 14:14 [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct Dave Martin
2011-07-26 14:16 ` Dave Martin
@ 2011-07-26 16:29 ` Kirill A. Shutemov
2011-07-26 17:16 ` Dave Martin
2011-07-26 20:18 ` Kirill A. Shutemov
2011-07-26 20:34 ` Nicolas Pitre
3 siblings, 1 reply; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-07-26 16:29 UTC (permalink / raw)
To: linux-arm-kernel
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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Kirill A. Shutemov
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct
2011-07-26 16:29 ` Kirill A. Shutemov
@ 2011-07-26 17:16 ` Dave Martin
2011-07-26 17:48 ` Kirill A. Shutemov
0 siblings, 1 reply; 7+ messages in thread
From: Dave Martin @ 2011-07-26 17:16 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct
2011-07-26 14:14 [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct Dave Martin
2011-07-26 14:16 ` Dave Martin
2011-07-26 16:29 ` Kirill A. Shutemov
@ 2011-07-26 20:18 ` Kirill A. Shutemov
2011-07-26 20:34 ` Nicolas Pitre
3 siblings, 0 replies; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-07-26 20:18 UTC (permalink / raw)
To: linux-arm-kernel
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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Kirill A. Shutemov
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct
2011-07-26 14:14 [PATCH 1/2] ARM: alignment: Make SIGBUS sent to userspace POSIXly correct Dave Martin
` (2 preceding siblings ...)
2011-07-26 20:18 ` Kirill A. Shutemov
@ 2011-07-26 20:34 ` Nicolas Pitre
3 siblings, 0 replies; 7+ messages in thread
From: Nicolas Pitre @ 2011-07-26 20:34 UTC (permalink / raw)
To: 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread