* [PATCH v2 0/3] arm64: Fix invalid si_codes for fault signals @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Will Deacon, James Morse, Eric W. Biederman, Catalin Marinas Changes since v1: * Fixed failing BUILD_BUG_ON() sanity check on NSIGFPE done by x86. (Thanks to James Morse for spotting.) The addition of FPE_FLTUNK is harmless to x86, since only arm64 uses this code right now. Changes since RFC v2: * Fold in reverts of the relevant parts of Eric's *_FIXME changes, since this series removes all cases of those dummy si_code values from arm64. * Propagated the new signal/si_code assignments for synchronous external aborts correctly to signals thrown from do_sea(). Original blurb: As reported by Eric Biederman, [1], [2], [3] etc., several architectures are inappropriately setting si_code to 0 when signaling faults to userspace, which will interpret this value as SI_USER leading to garbage being read out of siginfo fields that the kernel doesn't initialise. This seems not to be a huge problem in practice, since many affected faults should only happen if the kernel or hardware is buggy or broken. However, there are some cases that don't fall under this. This RFC series proposes fixes to eliminate these si_code == 0 cases from arm64. Some of the changes may be controversial. There are two main headlines here, which may be applicable to other architectures: * addition of a new code FPE_UNKNOWN for undiagnosable floating-point exception traps; * delivering SIGKILL instead of SIGSEGV/BUS/TRAP etc. for "impossible" exceptions that are not feasible to recover from and likely indicate a kernel or system bug or failure. In particular there is likely to be a fair amount of overlap between arm and arm64 here, but due to the longer history and evolution of the arm tree and 32-bit architecture I'm less confident in making correct judgements for that case. This series doesn't touch arm, for now. Boot-tested on Arm Juno r0. Floating-point exception traps tested on the Arm Fast Model. [1] [PATCH 00/11] siginfo fixes/cleanups esp SI_USER https://marc.info/?l=linux-kernel&m=151571871109016&w=2 [2] [PATCH 08/11] signal/arm: Document conflicts with SI_USER and SIGFPE http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553574.html [3] [PATCH 07/11] signal/arm64: Document conflicts with SI_USER and SIGFPE, SIGTRAP, SIGBUS lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553573.html Dave Martin (3): signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE arm64: signal: Ensure si_code is valid for all fault signals arch/arm64/include/asm/esr.h | 9 +++ arch/arm64/include/uapi/asm/siginfo.h | 21 ------ arch/arm64/kernel/fpsimd.c | 31 +++++---- arch/arm64/mm/fault.c | 116 +++++++++++++++++----------------- arch/x86/kernel/signal_compat.c | 2 +- include/uapi/asm-generic/siginfo.h | 3 +- kernel/signal.c | 4 -- 7 files changed, 87 insertions(+), 99 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 0/3] arm64: Fix invalid si_codes for fault signals @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Changes since v1: * Fixed failing BUILD_BUG_ON() sanity check on NSIGFPE done by x86. (Thanks to James Morse for spotting.) The addition of FPE_FLTUNK is harmless to x86, since only arm64 uses this code right now. Changes since RFC v2: * Fold in reverts of the relevant parts of Eric's *_FIXME changes, since this series removes all cases of those dummy si_code values from arm64. * Propagated the new signal/si_code assignments for synchronous external aborts correctly to signals thrown from do_sea(). Original blurb: As reported by Eric Biederman, [1], [2], [3] etc., several architectures are inappropriately setting si_code to 0 when signaling faults to userspace, which will interpret this value as SI_USER leading to garbage being read out of siginfo fields that the kernel doesn't initialise. This seems not to be a huge problem in practice, since many affected faults should only happen if the kernel or hardware is buggy or broken. However, there are some cases that don't fall under this. This RFC series proposes fixes to eliminate these si_code == 0 cases from arm64. Some of the changes may be controversial. There are two main headlines here, which may be applicable to other architectures: * addition of a new code FPE_UNKNOWN for undiagnosable floating-point exception traps; * delivering SIGKILL instead of SIGSEGV/BUS/TRAP etc. for "impossible" exceptions that are not feasible to recover from and likely indicate a kernel or system bug or failure. In particular there is likely to be a fair amount of overlap between arm and arm64 here, but due to the longer history and evolution of the arm tree and 32-bit architecture I'm less confident in making correct judgements for that case. This series doesn't touch arm, for now. Boot-tested on Arm Juno r0. Floating-point exception traps tested on the Arm Fast Model. [1] [PATCH 00/11] siginfo fixes/cleanups esp SI_USER https://marc.info/?l=linux-kernel&m=151571871109016&w=2 [2] [PATCH 08/11] signal/arm: Document conflicts with SI_USER and SIGFPE http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553574.html [3] [PATCH 07/11] signal/arm64: Document conflicts with SI_USER and SIGFPE, SIGTRAP, SIGBUS lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553573.html Dave Martin (3): signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE arm64: signal: Ensure si_code is valid for all fault signals arch/arm64/include/asm/esr.h | 9 +++ arch/arm64/include/uapi/asm/siginfo.h | 21 ------ arch/arm64/kernel/fpsimd.c | 31 +++++---- arch/arm64/mm/fault.c | 116 +++++++++++++++++----------------- arch/x86/kernel/signal_compat.c | 2 +- include/uapi/asm-generic/siginfo.h | 3 +- kernel/signal.c | 4 -- 7 files changed, 87 insertions(+), 99 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 0/3] arm64: Fix invalid si_codes for fault signals @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Eric W. Biederman, Will Deacon, Catalin Marinas, James Morse Changes since v1: * Fixed failing BUILD_BUG_ON() sanity check on NSIGFPE done by x86. (Thanks to James Morse for spotting.) The addition of FPE_FLTUNK is harmless to x86, since only arm64 uses this code right now. Changes since RFC v2: * Fold in reverts of the relevant parts of Eric's *_FIXME changes, since this series removes all cases of those dummy si_code values from arm64. * Propagated the new signal/si_code assignments for synchronous external aborts correctly to signals thrown from do_sea(). Original blurb: As reported by Eric Biederman, [1], [2], [3] etc., several architectures are inappropriately setting si_code to 0 when signaling faults to userspace, which will interpret this value as SI_USER leading to garbage being read out of siginfo fields that the kernel doesn't initialise. This seems not to be a huge problem in practice, since many affected faults should only happen if the kernel or hardware is buggy or broken. However, there are some cases that don't fall under this. This RFC series proposes fixes to eliminate these si_code == 0 cases from arm64. Some of the changes may be controversial. There are two main headlines here, which may be applicable to other architectures: * addition of a new code FPE_UNKNOWN for undiagnosable floating-point exception traps; * delivering SIGKILL instead of SIGSEGV/BUS/TRAP etc. for "impossible" exceptions that are not feasible to recover from and likely indicate a kernel or system bug or failure. In particular there is likely to be a fair amount of overlap between arm and arm64 here, but due to the longer history and evolution of the arm tree and 32-bit architecture I'm less confident in making correct judgements for that case. This series doesn't touch arm, for now. Boot-tested on Arm Juno r0. Floating-point exception traps tested on the Arm Fast Model. [1] [PATCH 00/11] siginfo fixes/cleanups esp SI_USER https://marc.info/?l=linux-kernel&m=151571871109016&w=2 [2] [PATCH 08/11] signal/arm: Document conflicts with SI_USER and SIGFPE http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553574.html [3] [PATCH 07/11] signal/arm64: Document conflicts with SI_USER and SIGFPE, SIGTRAP, SIGBUS lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553573.html Dave Martin (3): signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE arm64: signal: Ensure si_code is valid for all fault signals arch/arm64/include/asm/esr.h | 9 +++ arch/arm64/include/uapi/asm/siginfo.h | 21 ------ arch/arm64/kernel/fpsimd.c | 31 +++++---- arch/arm64/mm/fault.c | 116 +++++++++++++++++----------------- arch/x86/kernel/signal_compat.c | 2 +- include/uapi/asm-generic/siginfo.h | 3 +- kernel/signal.c | 4 -- 7 files changed, 87 insertions(+), 99 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Will Deacon, James Morse, Eric W. Biederman, Catalin Marinas Some architectures cannot always report accurately what kind of floating-point exception triggered a floating-point exception trap. This can occur with fp exceptions occurring on lanes in a vector instruction on arm64 for example. Rather than have every architecture come up with its own way of describing such a condition, this patch adds a common FPE_FLTUNK si_code value to report that an fp exception caused a trap but we cannot be certain which kind of fp exception it was. Signed-off-by: Dave Martin <Dave.Martin@arm.com> -- Changes since v1: Reported by James Morse: * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. FPE_FLTUNK does not current have any implications for x86, since it is not currently used and has no implications for the way siginfo is populated. --- arch/x86/kernel/signal_compat.c | 2 +- include/uapi/asm-generic/siginfo.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c index ac057f9..954ad99 100644 --- a/arch/x86/kernel/signal_compat.c +++ b/arch/x86/kernel/signal_compat.c @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) * new fields are handled in copy_siginfo_to_user32()! */ BUILD_BUG_ON(NSIGILL != 11); - BUILD_BUG_ON(NSIGFPE != 13); + BUILD_BUG_ON(NSIGFPE != 14); BUILD_BUG_ON(NSIGSEGV != 4); BUILD_BUG_ON(NSIGBUS != 5); BUILD_BUG_ON(NSIGTRAP != 4); diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index 85dc965..10304de 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -229,7 +229,8 @@ typedef struct siginfo { # define __FPE_INVASC 12 /* invalid ASCII digit */ # define __FPE_INVDEC 13 /* invalid decimal digit */ #endif -#define NSIGFPE 13 +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ +#define NSIGFPE 14 /* * SIGSEGV si_codes -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Some architectures cannot always report accurately what kind of floating-point exception triggered a floating-point exception trap. This can occur with fp exceptions occurring on lanes in a vector instruction on arm64 for example. Rather than have every architecture come up with its own way of describing such a condition, this patch adds a common FPE_FLTUNK si_code value to report that an fp exception caused a trap but we cannot be certain which kind of fp exception it was. Signed-off-by: Dave Martin <Dave.Martin@arm.com> -- Changes since v1: Reported by James Morse: * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. FPE_FLTUNK does not current have any implications for x86, since it is not currently used and has no implications for the way siginfo is populated. --- arch/x86/kernel/signal_compat.c | 2 +- include/uapi/asm-generic/siginfo.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c index ac057f9..954ad99 100644 --- a/arch/x86/kernel/signal_compat.c +++ b/arch/x86/kernel/signal_compat.c @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) * new fields are handled in copy_siginfo_to_user32()! */ BUILD_BUG_ON(NSIGILL != 11); - BUILD_BUG_ON(NSIGFPE != 13); + BUILD_BUG_ON(NSIGFPE != 14); BUILD_BUG_ON(NSIGSEGV != 4); BUILD_BUG_ON(NSIGBUS != 5); BUILD_BUG_ON(NSIGTRAP != 4); diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index 85dc965..10304de 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -229,7 +229,8 @@ typedef struct siginfo { # define __FPE_INVASC 12 /* invalid ASCII digit */ # define __FPE_INVDEC 13 /* invalid decimal digit */ #endif -#define NSIGFPE 13 +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ +#define NSIGFPE 14 /* * SIGSEGV si_codes -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Eric W. Biederman, Will Deacon, Catalin Marinas, James Morse Some architectures cannot always report accurately what kind of floating-point exception triggered a floating-point exception trap. This can occur with fp exceptions occurring on lanes in a vector instruction on arm64 for example. Rather than have every architecture come up with its own way of describing such a condition, this patch adds a common FPE_FLTUNK si_code value to report that an fp exception caused a trap but we cannot be certain which kind of fp exception it was. Signed-off-by: Dave Martin <Dave.Martin@arm.com> -- Changes since v1: Reported by James Morse: * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. FPE_FLTUNK does not current have any implications for x86, since it is not currently used and has no implications for the way siginfo is populated. --- arch/x86/kernel/signal_compat.c | 2 +- include/uapi/asm-generic/siginfo.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c index ac057f9..954ad99 100644 --- a/arch/x86/kernel/signal_compat.c +++ b/arch/x86/kernel/signal_compat.c @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) * new fields are handled in copy_siginfo_to_user32()! */ BUILD_BUG_ON(NSIGILL != 11); - BUILD_BUG_ON(NSIGFPE != 13); + BUILD_BUG_ON(NSIGFPE != 14); BUILD_BUG_ON(NSIGSEGV != 4); BUILD_BUG_ON(NSIGBUS != 5); BUILD_BUG_ON(NSIGTRAP != 4); diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index 85dc965..10304de 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -229,7 +229,8 @@ typedef struct siginfo { # define __FPE_INVASC 12 /* invalid ASCII digit */ # define __FPE_INVDEC 13 /* invalid decimal digit */ #endif -#define NSIGFPE 13 +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ +#define NSIGFPE 14 /* * SIGSEGV si_codes -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions 2018-03-01 17:44 ` Dave Martin @ 2018-03-08 17:11 ` Will Deacon -1 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 17:11 UTC (permalink / raw) To: Dave Martin Cc: linux-arm-kernel, linux-arch, linux-api, Eric W. Biederman, Catalin Marinas, James Morse, linux-kernel, tglx, mingo, arnd Hi Dave, On Thu, Mar 01, 2018 at 05:44:06PM +0000, Dave Martin wrote: > Some architectures cannot always report accurately what kind of > floating-point exception triggered a floating-point exception trap. > > This can occur with fp exceptions occurring on lanes in a vector > instruction on arm64 for example. > > Rather than have every architecture come up with its own way of > describing such a condition, this patch adds a common FPE_FLTUNK > si_code value to report that an fp exception caused a trap but we > cannot be certain which kind of fp exception it was. This looks straightforward to me, but I'll need Acks for the x86 and uapi changes if I'm to take this via arm64. I've added some others to CC and kept the patch intact below. Cheers, Will > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > -- > > Changes since v1: > > Reported by James Morse: > > * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. > > FPE_FLTUNK does not current have any implications for x86, since it > is not currently used and has no implications for the way siginfo > is populated. > --- > arch/x86/kernel/signal_compat.c | 2 +- > include/uapi/asm-generic/siginfo.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c > index ac057f9..954ad99 100644 > --- a/arch/x86/kernel/signal_compat.c > +++ b/arch/x86/kernel/signal_compat.c > @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) > * new fields are handled in copy_siginfo_to_user32()! > */ > BUILD_BUG_ON(NSIGILL != 11); > - BUILD_BUG_ON(NSIGFPE != 13); > + BUILD_BUG_ON(NSIGFPE != 14); > BUILD_BUG_ON(NSIGSEGV != 4); > BUILD_BUG_ON(NSIGBUS != 5); > BUILD_BUG_ON(NSIGTRAP != 4); > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 85dc965..10304de 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -229,7 +229,8 @@ typedef struct siginfo { > # define __FPE_INVASC 12 /* invalid ASCII digit */ > # define __FPE_INVDEC 13 /* invalid decimal digit */ > #endif > -#define NSIGFPE 13 > +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ > +#define NSIGFPE 14 > > /* > * SIGSEGV si_codes > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-08 17:11 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 17:11 UTC (permalink / raw) To: linux-arm-kernel Hi Dave, On Thu, Mar 01, 2018 at 05:44:06PM +0000, Dave Martin wrote: > Some architectures cannot always report accurately what kind of > floating-point exception triggered a floating-point exception trap. > > This can occur with fp exceptions occurring on lanes in a vector > instruction on arm64 for example. > > Rather than have every architecture come up with its own way of > describing such a condition, this patch adds a common FPE_FLTUNK > si_code value to report that an fp exception caused a trap but we > cannot be certain which kind of fp exception it was. This looks straightforward to me, but I'll need Acks for the x86 and uapi changes if I'm to take this via arm64. I've added some others to CC and kept the patch intact below. Cheers, Will > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > -- > > Changes since v1: > > Reported by James Morse: > > * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. > > FPE_FLTUNK does not current have any implications for x86, since it > is not currently used and has no implications for the way siginfo > is populated. > --- > arch/x86/kernel/signal_compat.c | 2 +- > include/uapi/asm-generic/siginfo.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c > index ac057f9..954ad99 100644 > --- a/arch/x86/kernel/signal_compat.c > +++ b/arch/x86/kernel/signal_compat.c > @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) > * new fields are handled in copy_siginfo_to_user32()! > */ > BUILD_BUG_ON(NSIGILL != 11); > - BUILD_BUG_ON(NSIGFPE != 13); > + BUILD_BUG_ON(NSIGFPE != 14); > BUILD_BUG_ON(NSIGSEGV != 4); > BUILD_BUG_ON(NSIGBUS != 5); > BUILD_BUG_ON(NSIGTRAP != 4); > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 85dc965..10304de 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -229,7 +229,8 @@ typedef struct siginfo { > # define __FPE_INVASC 12 /* invalid ASCII digit */ > # define __FPE_INVDEC 13 /* invalid decimal digit */ > #endif > -#define NSIGFPE 13 > +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ > +#define NSIGFPE 14 > > /* > * SIGSEGV si_codes > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-08 22:35 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:35 UTC (permalink / raw) To: Dave Martin Cc: linux-arch, linux-api, Will Deacon, James Morse, Catalin Marinas, linux-arm-kernel Dave Martin <Dave.Martin@arm.com> writes: > Some architectures cannot always report accurately what kind of > floating-point exception triggered a floating-point exception trap. > > This can occur with fp exceptions occurring on lanes in a vector > instruction on arm64 for example. > > Rather than have every architecture come up with its own way of > describing such a condition, this patch adds a common FPE_FLTUNK > si_code value to report that an fp exception caused a trap but we > cannot be certain which kind of fp exception it was. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com> > -- > > Changes since v1: > > Reported by James Morse: > > * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. > > FPE_FLTUNK does not current have any implications for x86, since it > is not currently used and has no implications for the way siginfo > is populated. > --- > arch/x86/kernel/signal_compat.c | 2 +- > include/uapi/asm-generic/siginfo.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c > index ac057f9..954ad99 100644 > --- a/arch/x86/kernel/signal_compat.c > +++ b/arch/x86/kernel/signal_compat.c > @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) > * new fields are handled in copy_siginfo_to_user32()! > */ > BUILD_BUG_ON(NSIGILL != 11); > - BUILD_BUG_ON(NSIGFPE != 13); > + BUILD_BUG_ON(NSIGFPE != 14); > BUILD_BUG_ON(NSIGSEGV != 4); > BUILD_BUG_ON(NSIGBUS != 5); > BUILD_BUG_ON(NSIGTRAP != 4); For other reviewers. This bug on exists to ensure people remember to update the helper functions when new fields are added to struct siginfo. As new fields usually come with new si_codes. There are no fields added so this is sufficient. > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 85dc965..10304de 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -229,7 +229,8 @@ typedef struct siginfo { > # define __FPE_INVASC 12 /* invalid ASCII digit */ > # define __FPE_INVDEC 13 /* invalid decimal digit */ > #endif > -#define NSIGFPE 13 > +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ > +#define NSIGFPE 14 > > /* > * SIGSEGV si_codes ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-08 22:35 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:35 UTC (permalink / raw) To: linux-arm-kernel Dave Martin <Dave.Martin@arm.com> writes: > Some architectures cannot always report accurately what kind of > floating-point exception triggered a floating-point exception trap. > > This can occur with fp exceptions occurring on lanes in a vector > instruction on arm64 for example. > > Rather than have every architecture come up with its own way of > describing such a condition, this patch adds a common FPE_FLTUNK > si_code value to report that an fp exception caused a trap but we > cannot be certain which kind of fp exception it was. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com> > -- > > Changes since v1: > > Reported by James Morse: > > * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. > > FPE_FLTUNK does not current have any implications for x86, since it > is not currently used and has no implications for the way siginfo > is populated. > --- > arch/x86/kernel/signal_compat.c | 2 +- > include/uapi/asm-generic/siginfo.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c > index ac057f9..954ad99 100644 > --- a/arch/x86/kernel/signal_compat.c > +++ b/arch/x86/kernel/signal_compat.c > @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) > * new fields are handled in copy_siginfo_to_user32()! > */ > BUILD_BUG_ON(NSIGILL != 11); > - BUILD_BUG_ON(NSIGFPE != 13); > + BUILD_BUG_ON(NSIGFPE != 14); > BUILD_BUG_ON(NSIGSEGV != 4); > BUILD_BUG_ON(NSIGBUS != 5); > BUILD_BUG_ON(NSIGTRAP != 4); For other reviewers. This bug on exists to ensure people remember to update the helper functions when new fields are added to struct siginfo. As new fields usually come with new si_codes. There are no fields added so this is sufficient. > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 85dc965..10304de 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -229,7 +229,8 @@ typedef struct siginfo { > # define __FPE_INVASC 12 /* invalid ASCII digit */ > # define __FPE_INVDEC 13 /* invalid decimal digit */ > #endif > -#define NSIGFPE 13 > +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ > +#define NSIGFPE 14 > > /* > * SIGSEGV si_codes ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions @ 2018-03-08 22:35 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:35 UTC (permalink / raw) To: Dave Martin Cc: linux-arm-kernel, linux-arch, linux-api, Will Deacon, Catalin Marinas, James Morse Dave Martin <Dave.Martin@arm.com> writes: > Some architectures cannot always report accurately what kind of > floating-point exception triggered a floating-point exception trap. > > This can occur with fp exceptions occurring on lanes in a vector > instruction on arm64 for example. > > Rather than have every architecture come up with its own way of > describing such a condition, this patch adds a common FPE_FLTUNK > si_code value to report that an fp exception caused a trap but we > cannot be certain which kind of fp exception it was. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com> > -- > > Changes since v1: > > Reported by James Morse: > > * Bump NSIGFPE BUILD_BUG_ON() check for x86 compat. > > FPE_FLTUNK does not current have any implications for x86, since it > is not currently used and has no implications for the way siginfo > is populated. > --- > arch/x86/kernel/signal_compat.c | 2 +- > include/uapi/asm-generic/siginfo.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c > index ac057f9..954ad99 100644 > --- a/arch/x86/kernel/signal_compat.c > +++ b/arch/x86/kernel/signal_compat.c > @@ -26,7 +26,7 @@ static inline void signal_compat_build_tests(void) > * new fields are handled in copy_siginfo_to_user32()! > */ > BUILD_BUG_ON(NSIGILL != 11); > - BUILD_BUG_ON(NSIGFPE != 13); > + BUILD_BUG_ON(NSIGFPE != 14); > BUILD_BUG_ON(NSIGSEGV != 4); > BUILD_BUG_ON(NSIGBUS != 5); > BUILD_BUG_ON(NSIGTRAP != 4); For other reviewers. This bug on exists to ensure people remember to update the helper functions when new fields are added to struct siginfo. As new fields usually come with new si_codes. There are no fields added so this is sufficient. > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 85dc965..10304de 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -229,7 +229,8 @@ typedef struct siginfo { > # define __FPE_INVASC 12 /* invalid ASCII digit */ > # define __FPE_INVDEC 13 /* invalid decimal digit */ > #endif > -#define NSIGFPE 13 > +#define FPE_FLTUNK 14 /* undiagnosed floating-point exception */ > +#define NSIGFPE 14 > > /* > * SIGSEGV si_codes ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Will Deacon, James Morse, Eric W. Biederman, Catalin Marinas Currently a SIGFPE delivered in response to a floating-point exception trap may have si_code set to 0 on arm64. As reported by Eric, this is a bad idea since this is the value of SI_USER -- yet this signal is definitely not the result of kill(2), tgkill(2) etc. and si_uid and si_pid make limited sense whereas we do want to yield a value for si_addr (which doesn't exist for SI_USER). It's not entirely clear whether the architecure permits a "spurious" fp exception trap where none of the exception flag bits in ESR_ELx is set. (IMHO the architectural intent is to forbid this.) However, it does permit those bits to contain garbage if the TFV bit in ESR_ELx is 0. That case isn't currently handled at all and may result in si_code == 0 or si_code containing a FPE_FLT* constant corresponding to an exception that did not in fact happen. There is nothing sensible we can return for si_code in such cases, but SI_USER is certainly not appropriate and will lead to violation of legitimate userspace assumptions. This patch allocates a new si_code value FPE_UNKNOWN that at least does not conflict with any existing SI_* or FPE_* code, and yields this in si_code for undiagnosable cases. This is probably the best simplicity/incorrectness tradeoff achieveable without relying on implementation-dependent features or adding a lot of code. In any case, there appears to be no perfect solution possible that would justify a lot of effort here. Yielding FPE_UNKNOWN when some well-defined fp exception caused the trap is a violation of POSIX, but this is forced by the architecture. We have no realistic prospect of yielding the correct code in such cases. At present I am not aware of any ARMv8 implementation that supports trapped floating-point exceptions in any case. The new code may be applicable to other architectures for similar reasons. No attempt is made to provide ESR_ELx to userspace in the signal frame, since architectural limitations mean that it is unlikely to provide much diagnostic value, doesn't benefit existing software and would create ABI with no proven purpose. The existing mechanism for passing it also has problems of its own which may result in the wrong value being passed to userspace due to interaction with mm faults. The implied rework does not appear justified. Reported-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/include/asm/esr.h | 9 +++++++++ arch/arm64/include/uapi/asm/siginfo.h | 7 ------- arch/arm64/kernel/fpsimd.c | 27 +++++++++++++++------------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 803443d..ce70c3f 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -240,6 +240,15 @@ (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ ESR_ELx_SYS64_ISS_OP2_SHIFT)) +/* + * ISS field definitions for floating-point exception traps + * (FP_EXC_32/FP_EXC_64). + * + * (The FPEXC_* constants are used instead for common bits.) + */ + +#define ESR_ELx_FP_EXC_TFV (UL(1) << 23) + #ifndef __ASSEMBLY__ #include <asm/types.h> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 9b4d912..157e6a8 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -22,13 +22,6 @@ #include <asm-generic/siginfo.h> /* - * SIGFPE si_codes - */ -#ifdef __KERNEL__ -#define FPE_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - -/* * SIGBUS si_codes */ #ifdef __KERNEL__ diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index e7226c4..9040038 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/sysctl.h> +#include <asm/esr.h> #include <asm/fpsimd.h> #include <asm/cputype.h> #include <asm/simd.h> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) { siginfo_t info; - unsigned int si_code = FPE_FIXME; - - if (esr & FPEXC_IOF) - si_code = FPE_FLTINV; - else if (esr & FPEXC_DZF) - si_code = FPE_FLTDIV; - else if (esr & FPEXC_OFF) - si_code = FPE_FLTOVF; - else if (esr & FPEXC_UFF) - si_code = FPE_FLTUND; - else if (esr & FPEXC_IXF) - si_code = FPE_FLTRES; + unsigned int si_code = FPE_FLTUNK; + + if (esr & ESR_ELx_FP_EXC_TFV) { + if (esr & FPEXC_IOF) + si_code = FPE_FLTINV; + else if (esr & FPEXC_DZF) + si_code = FPE_FLTDIV; + else if (esr & FPEXC_OFF) + si_code = FPE_FLTOVF; + else if (esr & FPEXC_UFF) + si_code = FPE_FLTUND; + else if (esr & FPEXC_IXF) + si_code = FPE_FLTRES; + } memset(&info, 0, sizeof(info)); info.si_signo = SIGFPE; -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Currently a SIGFPE delivered in response to a floating-point exception trap may have si_code set to 0 on arm64. As reported by Eric, this is a bad idea since this is the value of SI_USER -- yet this signal is definitely not the result of kill(2), tgkill(2) etc. and si_uid and si_pid make limited sense whereas we do want to yield a value for si_addr (which doesn't exist for SI_USER). It's not entirely clear whether the architecure permits a "spurious" fp exception trap where none of the exception flag bits in ESR_ELx is set. (IMHO the architectural intent is to forbid this.) However, it does permit those bits to contain garbage if the TFV bit in ESR_ELx is 0. That case isn't currently handled at all and may result in si_code == 0 or si_code containing a FPE_FLT* constant corresponding to an exception that did not in fact happen. There is nothing sensible we can return for si_code in such cases, but SI_USER is certainly not appropriate and will lead to violation of legitimate userspace assumptions. This patch allocates a new si_code value FPE_UNKNOWN that at least does not conflict with any existing SI_* or FPE_* code, and yields this in si_code for undiagnosable cases. This is probably the best simplicity/incorrectness tradeoff achieveable without relying on implementation-dependent features or adding a lot of code. In any case, there appears to be no perfect solution possible that would justify a lot of effort here. Yielding FPE_UNKNOWN when some well-defined fp exception caused the trap is a violation of POSIX, but this is forced by the architecture. We have no realistic prospect of yielding the correct code in such cases. At present I am not aware of any ARMv8 implementation that supports trapped floating-point exceptions in any case. The new code may be applicable to other architectures for similar reasons. No attempt is made to provide ESR_ELx to userspace in the signal frame, since architectural limitations mean that it is unlikely to provide much diagnostic value, doesn't benefit existing software and would create ABI with no proven purpose. The existing mechanism for passing it also has problems of its own which may result in the wrong value being passed to userspace due to interaction with mm faults. The implied rework does not appear justified. Reported-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/include/asm/esr.h | 9 +++++++++ arch/arm64/include/uapi/asm/siginfo.h | 7 ------- arch/arm64/kernel/fpsimd.c | 27 +++++++++++++++------------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 803443d..ce70c3f 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -240,6 +240,15 @@ (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ ESR_ELx_SYS64_ISS_OP2_SHIFT)) +/* + * ISS field definitions for floating-point exception traps + * (FP_EXC_32/FP_EXC_64). + * + * (The FPEXC_* constants are used instead for common bits.) + */ + +#define ESR_ELx_FP_EXC_TFV (UL(1) << 23) + #ifndef __ASSEMBLY__ #include <asm/types.h> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 9b4d912..157e6a8 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -22,13 +22,6 @@ #include <asm-generic/siginfo.h> /* - * SIGFPE si_codes - */ -#ifdef __KERNEL__ -#define FPE_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - -/* * SIGBUS si_codes */ #ifdef __KERNEL__ diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index e7226c4..9040038 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/sysctl.h> +#include <asm/esr.h> #include <asm/fpsimd.h> #include <asm/cputype.h> #include <asm/simd.h> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) { siginfo_t info; - unsigned int si_code = FPE_FIXME; - - if (esr & FPEXC_IOF) - si_code = FPE_FLTINV; - else if (esr & FPEXC_DZF) - si_code = FPE_FLTDIV; - else if (esr & FPEXC_OFF) - si_code = FPE_FLTOVF; - else if (esr & FPEXC_UFF) - si_code = FPE_FLTUND; - else if (esr & FPEXC_IXF) - si_code = FPE_FLTRES; + unsigned int si_code = FPE_FLTUNK; + + if (esr & ESR_ELx_FP_EXC_TFV) { + if (esr & FPEXC_IOF) + si_code = FPE_FLTINV; + else if (esr & FPEXC_DZF) + si_code = FPE_FLTDIV; + else if (esr & FPEXC_OFF) + si_code = FPE_FLTOVF; + else if (esr & FPEXC_UFF) + si_code = FPE_FLTUND; + else if (esr & FPEXC_IXF) + si_code = FPE_FLTRES; + } memset(&info, 0, sizeof(info)); info.si_signo = SIGFPE; -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Eric W. Biederman, Will Deacon, Catalin Marinas, James Morse Currently a SIGFPE delivered in response to a floating-point exception trap may have si_code set to 0 on arm64. As reported by Eric, this is a bad idea since this is the value of SI_USER -- yet this signal is definitely not the result of kill(2), tgkill(2) etc. and si_uid and si_pid make limited sense whereas we do want to yield a value for si_addr (which doesn't exist for SI_USER). It's not entirely clear whether the architecure permits a "spurious" fp exception trap where none of the exception flag bits in ESR_ELx is set. (IMHO the architectural intent is to forbid this.) However, it does permit those bits to contain garbage if the TFV bit in ESR_ELx is 0. That case isn't currently handled at all and may result in si_code == 0 or si_code containing a FPE_FLT* constant corresponding to an exception that did not in fact happen. There is nothing sensible we can return for si_code in such cases, but SI_USER is certainly not appropriate and will lead to violation of legitimate userspace assumptions. This patch allocates a new si_code value FPE_UNKNOWN that at least does not conflict with any existing SI_* or FPE_* code, and yields this in si_code for undiagnosable cases. This is probably the best simplicity/incorrectness tradeoff achieveable without relying on implementation-dependent features or adding a lot of code. In any case, there appears to be no perfect solution possible that would justify a lot of effort here. Yielding FPE_UNKNOWN when some well-defined fp exception caused the trap is a violation of POSIX, but this is forced by the architecture. We have no realistic prospect of yielding the correct code in such cases. At present I am not aware of any ARMv8 implementation that supports trapped floating-point exceptions in any case. The new code may be applicable to other architectures for similar reasons. No attempt is made to provide ESR_ELx to userspace in the signal frame, since architectural limitations mean that it is unlikely to provide much diagnostic value, doesn't benefit existing software and would create ABI with no proven purpose. The existing mechanism for passing it also has problems of its own which may result in the wrong value being passed to userspace due to interaction with mm faults. The implied rework does not appear justified. Reported-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/include/asm/esr.h | 9 +++++++++ arch/arm64/include/uapi/asm/siginfo.h | 7 ------- arch/arm64/kernel/fpsimd.c | 27 +++++++++++++++------------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 803443d..ce70c3f 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -240,6 +240,15 @@ (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ ESR_ELx_SYS64_ISS_OP2_SHIFT)) +/* + * ISS field definitions for floating-point exception traps + * (FP_EXC_32/FP_EXC_64). + * + * (The FPEXC_* constants are used instead for common bits.) + */ + +#define ESR_ELx_FP_EXC_TFV (UL(1) << 23) + #ifndef __ASSEMBLY__ #include <asm/types.h> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 9b4d912..157e6a8 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -22,13 +22,6 @@ #include <asm-generic/siginfo.h> /* - * SIGFPE si_codes - */ -#ifdef __KERNEL__ -#define FPE_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - -/* * SIGBUS si_codes */ #ifdef __KERNEL__ diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index e7226c4..9040038 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/sysctl.h> +#include <asm/esr.h> #include <asm/fpsimd.h> #include <asm/cputype.h> #include <asm/simd.h> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) { siginfo_t info; - unsigned int si_code = FPE_FIXME; - - if (esr & FPEXC_IOF) - si_code = FPE_FLTINV; - else if (esr & FPEXC_DZF) - si_code = FPE_FLTDIV; - else if (esr & FPEXC_OFF) - si_code = FPE_FLTOVF; - else if (esr & FPEXC_UFF) - si_code = FPE_FLTUND; - else if (esr & FPEXC_IXF) - si_code = FPE_FLTRES; + unsigned int si_code = FPE_FLTUNK; + + if (esr & ESR_ELx_FP_EXC_TFV) { + if (esr & FPEXC_IOF) + si_code = FPE_FLTINV; + else if (esr & FPEXC_DZF) + si_code = FPE_FLTDIV; + else if (esr & FPEXC_OFF) + si_code = FPE_FLTOVF; + else if (esr & FPEXC_UFF) + si_code = FPE_FLTUND; + else if (esr & FPEXC_IXF) + si_code = FPE_FLTRES; + } memset(&info, 0, sizeof(info)); info.si_signo = SIGFPE; -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 17:11 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 17:11 UTC (permalink / raw) To: Dave Martin Cc: linux-arch, linux-api, James Morse, Eric W. Biederman, Catalin Marinas, linux-arm-kernel On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > Currently a SIGFPE delivered in response to a floating-point > exception trap may have si_code set to 0 on arm64. As reported by > Eric, this is a bad idea since this is the value of SI_USER -- yet > this signal is definitely not the result of kill(2), tgkill(2) etc. > and si_uid and si_pid make limited sense whereas we do want to > yield a value for si_addr (which doesn't exist for SI_USER). [...] > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index e7226c4..9040038 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -39,6 +39,7 @@ > #include <linux/slab.h> > #include <linux/sysctl.h> > > +#include <asm/esr.h> > #include <asm/fpsimd.h> > #include <asm/cputype.h> > #include <asm/simd.h> > @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > { > siginfo_t info; > - unsigned int si_code = FPE_FIXME; > - > - if (esr & FPEXC_IOF) > - si_code = FPE_FLTINV; > - else if (esr & FPEXC_DZF) > - si_code = FPE_FLTDIV; > - else if (esr & FPEXC_OFF) > - si_code = FPE_FLTOVF; > - else if (esr & FPEXC_UFF) > - si_code = FPE_FLTUND; > - else if (esr & FPEXC_IXF) > - si_code = FPE_FLTRES; > + unsigned int si_code = FPE_FLTUNK; Happy to take this patch once the dependency on FPE_FLTUNK in core code is resolved. Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 17:11 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 17:11 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > Currently a SIGFPE delivered in response to a floating-point > exception trap may have si_code set to 0 on arm64. As reported by > Eric, this is a bad idea since this is the value of SI_USER -- yet > this signal is definitely not the result of kill(2), tgkill(2) etc. > and si_uid and si_pid make limited sense whereas we do want to > yield a value for si_addr (which doesn't exist for SI_USER). [...] > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index e7226c4..9040038 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -39,6 +39,7 @@ > #include <linux/slab.h> > #include <linux/sysctl.h> > > +#include <asm/esr.h> > #include <asm/fpsimd.h> > #include <asm/cputype.h> > #include <asm/simd.h> > @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > { > siginfo_t info; > - unsigned int si_code = FPE_FIXME; > - > - if (esr & FPEXC_IOF) > - si_code = FPE_FLTINV; > - else if (esr & FPEXC_DZF) > - si_code = FPE_FLTDIV; > - else if (esr & FPEXC_OFF) > - si_code = FPE_FLTOVF; > - else if (esr & FPEXC_UFF) > - si_code = FPE_FLTUND; > - else if (esr & FPEXC_IXF) > - si_code = FPE_FLTRES; > + unsigned int si_code = FPE_FLTUNK; Happy to take this patch once the dependency on FPE_FLTUNK in core code is resolved. Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 17:11 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 17:11 UTC (permalink / raw) To: Dave Martin Cc: linux-arm-kernel, linux-arch, linux-api, Eric W. Biederman, Catalin Marinas, James Morse On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > Currently a SIGFPE delivered in response to a floating-point > exception trap may have si_code set to 0 on arm64. As reported by > Eric, this is a bad idea since this is the value of SI_USER -- yet > this signal is definitely not the result of kill(2), tgkill(2) etc. > and si_uid and si_pid make limited sense whereas we do want to > yield a value for si_addr (which doesn't exist for SI_USER). [...] > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index e7226c4..9040038 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -39,6 +39,7 @@ > #include <linux/slab.h> > #include <linux/sysctl.h> > > +#include <asm/esr.h> > #include <asm/fpsimd.h> > #include <asm/cputype.h> > #include <asm/simd.h> > @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > { > siginfo_t info; > - unsigned int si_code = FPE_FIXME; > - > - if (esr & FPEXC_IOF) > - si_code = FPE_FLTINV; > - else if (esr & FPEXC_DZF) > - si_code = FPE_FLTDIV; > - else if (esr & FPEXC_OFF) > - si_code = FPE_FLTOVF; > - else if (esr & FPEXC_UFF) > - si_code = FPE_FLTUND; > - else if (esr & FPEXC_IXF) > - si_code = FPE_FLTRES; > + unsigned int si_code = FPE_FLTUNK; Happy to take this patch once the dependency on FPE_FLTUNK in core code is resolved. Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 22:40 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:40 UTC (permalink / raw) To: Will Deacon Cc: linux-arch, linux-api, James Morse, Catalin Marinas, Dave Martin, linux-arm-kernel Will Deacon <will.deacon@arm.com> writes: > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: >> Currently a SIGFPE delivered in response to a floating-point >> exception trap may have si_code set to 0 on arm64. As reported by >> Eric, this is a bad idea since this is the value of SI_USER -- yet >> this signal is definitely not the result of kill(2), tgkill(2) etc. >> and si_uid and si_pid make limited sense whereas we do want to >> yield a value for si_addr (which doesn't exist for SI_USER). > > [...] > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index e7226c4..9040038 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -39,6 +39,7 @@ >> #include <linux/slab.h> >> #include <linux/sysctl.h> >> >> +#include <asm/esr.h> >> #include <asm/fpsimd.h> >> #include <asm/cputype.h> >> #include <asm/simd.h> >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) >> { >> siginfo_t info; >> - unsigned int si_code = FPE_FIXME; >> - >> - if (esr & FPEXC_IOF) >> - si_code = FPE_FLTINV; >> - else if (esr & FPEXC_DZF) >> - si_code = FPE_FLTDIV; >> - else if (esr & FPEXC_OFF) >> - si_code = FPE_FLTOVF; >> - else if (esr & FPEXC_UFF) >> - si_code = FPE_FLTUND; >> - else if (esr & FPEXC_IXF) >> - si_code = FPE_FLTRES; >> + unsigned int si_code = FPE_FLTUNK; > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > resolved. Would it help for me to take the FPE_FLTUNK patch into my siginfo-next branch? So that there is a common branch with the code so we don't need to worry about conflicts. If so I will look at that on Monday. There is another FPE code that appears to be needed for one of the other architectures as well. If we can figure out how to head conflicts off at the pass while everyone is fixing the FPE_FIXME type problems it might help. But it is all simple enough I expect a word to the wise when I send Linus my pull request will be enough to sort things out. Eric ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 22:40 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:40 UTC (permalink / raw) To: linux-arm-kernel Will Deacon <will.deacon@arm.com> writes: > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: >> Currently a SIGFPE delivered in response to a floating-point >> exception trap may have si_code set to 0 on arm64. As reported by >> Eric, this is a bad idea since this is the value of SI_USER -- yet >> this signal is definitely not the result of kill(2), tgkill(2) etc. >> and si_uid and si_pid make limited sense whereas we do want to >> yield a value for si_addr (which doesn't exist for SI_USER). > > [...] > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index e7226c4..9040038 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -39,6 +39,7 @@ >> #include <linux/slab.h> >> #include <linux/sysctl.h> >> >> +#include <asm/esr.h> >> #include <asm/fpsimd.h> >> #include <asm/cputype.h> >> #include <asm/simd.h> >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) >> { >> siginfo_t info; >> - unsigned int si_code = FPE_FIXME; >> - >> - if (esr & FPEXC_IOF) >> - si_code = FPE_FLTINV; >> - else if (esr & FPEXC_DZF) >> - si_code = FPE_FLTDIV; >> - else if (esr & FPEXC_OFF) >> - si_code = FPE_FLTOVF; >> - else if (esr & FPEXC_UFF) >> - si_code = FPE_FLTUND; >> - else if (esr & FPEXC_IXF) >> - si_code = FPE_FLTRES; >> + unsigned int si_code = FPE_FLTUNK; > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > resolved. Would it help for me to take the FPE_FLTUNK patch into my siginfo-next branch? So that there is a common branch with the code so we don't need to worry about conflicts. If so I will look at that on Monday. There is another FPE code that appears to be needed for one of the other architectures as well. If we can figure out how to head conflicts off at the pass while everyone is fixing the FPE_FIXME type problems it might help. But it is all simple enough I expect a word to the wise when I send Linus my pull request will be enough to sort things out. Eric ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 22:40 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:40 UTC (permalink / raw) To: Will Deacon Cc: Dave Martin, linux-arm-kernel, linux-arch, linux-api, Catalin Marinas, James Morse Will Deacon <will.deacon@arm.com> writes: > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: >> Currently a SIGFPE delivered in response to a floating-point >> exception trap may have si_code set to 0 on arm64. As reported by >> Eric, this is a bad idea since this is the value of SI_USER -- yet >> this signal is definitely not the result of kill(2), tgkill(2) etc. >> and si_uid and si_pid make limited sense whereas we do want to >> yield a value for si_addr (which doesn't exist for SI_USER). > > [...] > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index e7226c4..9040038 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -39,6 +39,7 @@ >> #include <linux/slab.h> >> #include <linux/sysctl.h> >> >> +#include <asm/esr.h> >> #include <asm/fpsimd.h> >> #include <asm/cputype.h> >> #include <asm/simd.h> >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) >> { >> siginfo_t info; >> - unsigned int si_code = FPE_FIXME; >> - >> - if (esr & FPEXC_IOF) >> - si_code = FPE_FLTINV; >> - else if (esr & FPEXC_DZF) >> - si_code = FPE_FLTDIV; >> - else if (esr & FPEXC_OFF) >> - si_code = FPE_FLTOVF; >> - else if (esr & FPEXC_UFF) >> - si_code = FPE_FLTUND; >> - else if (esr & FPEXC_IXF) >> - si_code = FPE_FLTRES; >> + unsigned int si_code = FPE_FLTUNK; > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > resolved. Would it help for me to take the FPE_FLTUNK patch into my siginfo-next branch? So that there is a common branch with the code so we don't need to worry about conflicts. If so I will look at that on Monday. There is another FPE code that appears to be needed for one of the other architectures as well. If we can figure out how to head conflicts off at the pass while everyone is fixing the FPE_FIXME type problems it might help. But it is all simple enough I expect a word to the wise when I send Linus my pull request will be enough to sort things out. Eric ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-09 13:10 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-09 13:10 UTC (permalink / raw) To: Eric W. Biederman Cc: linux-arch, linux-api, James Morse, Catalin Marinas, Dave Martin, linux-arm-kernel Hi Eric, On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > Will Deacon <will.deacon@arm.com> writes: > > > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > >> Currently a SIGFPE delivered in response to a floating-point > >> exception trap may have si_code set to 0 on arm64. As reported by > >> Eric, this is a bad idea since this is the value of SI_USER -- yet > >> this signal is definitely not the result of kill(2), tgkill(2) etc. > >> and si_uid and si_pid make limited sense whereas we do want to > >> yield a value for si_addr (which doesn't exist for SI_USER). > > > > [...] > > > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > >> index e7226c4..9040038 100644 > >> --- a/arch/arm64/kernel/fpsimd.c > >> +++ b/arch/arm64/kernel/fpsimd.c > >> @@ -39,6 +39,7 @@ > >> #include <linux/slab.h> > >> #include <linux/sysctl.h> > >> > >> +#include <asm/esr.h> > >> #include <asm/fpsimd.h> > >> #include <asm/cputype.h> > >> #include <asm/simd.h> > >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > >> { > >> siginfo_t info; > >> - unsigned int si_code = FPE_FIXME; > >> - > >> - if (esr & FPEXC_IOF) > >> - si_code = FPE_FLTINV; > >> - else if (esr & FPEXC_DZF) > >> - si_code = FPE_FLTDIV; > >> - else if (esr & FPEXC_OFF) > >> - si_code = FPE_FLTOVF; > >> - else if (esr & FPEXC_UFF) > >> - si_code = FPE_FLTUND; > >> - else if (esr & FPEXC_IXF) > >> - si_code = FPE_FLTRES; > >> + unsigned int si_code = FPE_FLTUNK; > > > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > > resolved. > > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > branch? So that there is a common branch with the code so we don't > need to worry about conflicts. If so I will look at that on Monday. Yes please, that would be helpful actually. I can then pull that into arm64 if you give me a stable branch or tag. Alternatively, I can define FPE_FLTUNK locally and remove it at -rc1. Cheers, Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-09 13:10 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-09 13:10 UTC (permalink / raw) To: linux-arm-kernel Hi Eric, On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > Will Deacon <will.deacon@arm.com> writes: > > > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > >> Currently a SIGFPE delivered in response to a floating-point > >> exception trap may have si_code set to 0 on arm64. As reported by > >> Eric, this is a bad idea since this is the value of SI_USER -- yet > >> this signal is definitely not the result of kill(2), tgkill(2) etc. > >> and si_uid and si_pid make limited sense whereas we do want to > >> yield a value for si_addr (which doesn't exist for SI_USER). > > > > [...] > > > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > >> index e7226c4..9040038 100644 > >> --- a/arch/arm64/kernel/fpsimd.c > >> +++ b/arch/arm64/kernel/fpsimd.c > >> @@ -39,6 +39,7 @@ > >> #include <linux/slab.h> > >> #include <linux/sysctl.h> > >> > >> +#include <asm/esr.h> > >> #include <asm/fpsimd.h> > >> #include <asm/cputype.h> > >> #include <asm/simd.h> > >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > >> { > >> siginfo_t info; > >> - unsigned int si_code = FPE_FIXME; > >> - > >> - if (esr & FPEXC_IOF) > >> - si_code = FPE_FLTINV; > >> - else if (esr & FPEXC_DZF) > >> - si_code = FPE_FLTDIV; > >> - else if (esr & FPEXC_OFF) > >> - si_code = FPE_FLTOVF; > >> - else if (esr & FPEXC_UFF) > >> - si_code = FPE_FLTUND; > >> - else if (esr & FPEXC_IXF) > >> - si_code = FPE_FLTRES; > >> + unsigned int si_code = FPE_FLTUNK; > > > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > > resolved. > > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > branch? So that there is a common branch with the code so we don't > need to worry about conflicts. If so I will look at that on Monday. Yes please, that would be helpful actually. I can then pull that into arm64 if you give me a stable branch or tag. Alternatively, I can define FPE_FLTUNK locally and remove it at -rc1. Cheers, Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-09 13:10 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-09 13:10 UTC (permalink / raw) To: Eric W. Biederman Cc: Dave Martin, linux-arm-kernel, linux-arch, linux-api, Catalin Marinas, James Morse Hi Eric, On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > Will Deacon <will.deacon@arm.com> writes: > > > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > >> Currently a SIGFPE delivered in response to a floating-point > >> exception trap may have si_code set to 0 on arm64. As reported by > >> Eric, this is a bad idea since this is the value of SI_USER -- yet > >> this signal is definitely not the result of kill(2), tgkill(2) etc. > >> and si_uid and si_pid make limited sense whereas we do want to > >> yield a value for si_addr (which doesn't exist for SI_USER). > > > > [...] > > > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > >> index e7226c4..9040038 100644 > >> --- a/arch/arm64/kernel/fpsimd.c > >> +++ b/arch/arm64/kernel/fpsimd.c > >> @@ -39,6 +39,7 @@ > >> #include <linux/slab.h> > >> #include <linux/sysctl.h> > >> > >> +#include <asm/esr.h> > >> #include <asm/fpsimd.h> > >> #include <asm/cputype.h> > >> #include <asm/simd.h> > >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > >> { > >> siginfo_t info; > >> - unsigned int si_code = FPE_FIXME; > >> - > >> - if (esr & FPEXC_IOF) > >> - si_code = FPE_FLTINV; > >> - else if (esr & FPEXC_DZF) > >> - si_code = FPE_FLTDIV; > >> - else if (esr & FPEXC_OFF) > >> - si_code = FPE_FLTOVF; > >> - else if (esr & FPEXC_UFF) > >> - si_code = FPE_FLTUND; > >> - else if (esr & FPEXC_IXF) > >> - si_code = FPE_FLTRES; > >> + unsigned int si_code = FPE_FLTUNK; > > > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > > resolved. > > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > branch? So that there is a common branch with the code so we don't > need to worry about conflicts. If so I will look at that on Monday. Yes please, that would be helpful actually. I can then pull that into arm64 if you give me a stable branch or tag. Alternatively, I can define FPE_FLTUNK locally and remove it at -rc1. Cheers, Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-09 14:25 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-09 14:25 UTC (permalink / raw) To: Will Deacon Cc: linux-arch, linux-api, James Morse, Eric W. Biederman, Catalin Marinas, linux-arm-kernel On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: > Hi Eric, > > On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > > Will Deacon <will.deacon@arm.com> writes: > > > > > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > > >> Currently a SIGFPE delivered in response to a floating-point > > >> exception trap may have si_code set to 0 on arm64. As reported by > > >> Eric, this is a bad idea since this is the value of SI_USER -- yet > > >> this signal is definitely not the result of kill(2), tgkill(2) etc. > > >> and si_uid and si_pid make limited sense whereas we do want to > > >> yield a value for si_addr (which doesn't exist for SI_USER). > > > > > > [...] > > > > > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > > >> index e7226c4..9040038 100644 > > >> --- a/arch/arm64/kernel/fpsimd.c > > >> +++ b/arch/arm64/kernel/fpsimd.c > > >> @@ -39,6 +39,7 @@ > > >> #include <linux/slab.h> > > >> #include <linux/sysctl.h> > > >> > > >> +#include <asm/esr.h> > > >> #include <asm/fpsimd.h> > > >> #include <asm/cputype.h> > > >> #include <asm/simd.h> > > >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > > >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > > >> { > > >> siginfo_t info; > > >> - unsigned int si_code = FPE_FIXME; > > >> - > > >> - if (esr & FPEXC_IOF) > > >> - si_code = FPE_FLTINV; > > >> - else if (esr & FPEXC_DZF) > > >> - si_code = FPE_FLTDIV; > > >> - else if (esr & FPEXC_OFF) > > >> - si_code = FPE_FLTOVF; > > >> - else if (esr & FPEXC_UFF) > > >> - si_code = FPE_FLTUND; > > >> - else if (esr & FPEXC_IXF) > > >> - si_code = FPE_FLTRES; > > >> + unsigned int si_code = FPE_FLTUNK; > > > > > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > > > resolved. > > > > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > > branch? So that there is a common branch with the code so we don't > > need to worry about conflicts. If so I will look at that on Monday. > > Yes please, that would be helpful actually. I can then pull that into > arm64 if you give me a stable branch or tag. Alternatively, I can define > FPE_FLTUNK locally and remove it at -rc1. OK Eric, let me know if you need me to rebase anything for the first 2 patches. Cheers ---Dave ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-09 14:25 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-09 14:25 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: > Hi Eric, > > On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > > Will Deacon <will.deacon@arm.com> writes: > > > > > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > > >> Currently a SIGFPE delivered in response to a floating-point > > >> exception trap may have si_code set to 0 on arm64. As reported by > > >> Eric, this is a bad idea since this is the value of SI_USER -- yet > > >> this signal is definitely not the result of kill(2), tgkill(2) etc. > > >> and si_uid and si_pid make limited sense whereas we do want to > > >> yield a value for si_addr (which doesn't exist for SI_USER). > > > > > > [...] > > > > > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > > >> index e7226c4..9040038 100644 > > >> --- a/arch/arm64/kernel/fpsimd.c > > >> +++ b/arch/arm64/kernel/fpsimd.c > > >> @@ -39,6 +39,7 @@ > > >> #include <linux/slab.h> > > >> #include <linux/sysctl.h> > > >> > > >> +#include <asm/esr.h> > > >> #include <asm/fpsimd.h> > > >> #include <asm/cputype.h> > > >> #include <asm/simd.h> > > >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > > >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > > >> { > > >> siginfo_t info; > > >> - unsigned int si_code = FPE_FIXME; > > >> - > > >> - if (esr & FPEXC_IOF) > > >> - si_code = FPE_FLTINV; > > >> - else if (esr & FPEXC_DZF) > > >> - si_code = FPE_FLTDIV; > > >> - else if (esr & FPEXC_OFF) > > >> - si_code = FPE_FLTOVF; > > >> - else if (esr & FPEXC_UFF) > > >> - si_code = FPE_FLTUND; > > >> - else if (esr & FPEXC_IXF) > > >> - si_code = FPE_FLTRES; > > >> + unsigned int si_code = FPE_FLTUNK; > > > > > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > > > resolved. > > > > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > > branch? So that there is a common branch with the code so we don't > > need to worry about conflicts. If so I will look at that on Monday. > > Yes please, that would be helpful actually. I can then pull that into > arm64 if you give me a stable branch or tag. Alternatively, I can define > FPE_FLTUNK locally and remove it at -rc1. OK Eric, let me know if you need me to rebase anything for the first 2 patches. Cheers ---Dave ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-09 14:25 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-09 14:25 UTC (permalink / raw) To: Will Deacon Cc: Eric W. Biederman, linux-arm-kernel, linux-arch, linux-api, Catalin Marinas, James Morse On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: > Hi Eric, > > On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > > Will Deacon <will.deacon@arm.com> writes: > > > > > On Thu, Mar 01, 2018 at 05:44:07PM +0000, Dave Martin wrote: > > >> Currently a SIGFPE delivered in response to a floating-point > > >> exception trap may have si_code set to 0 on arm64. As reported by > > >> Eric, this is a bad idea since this is the value of SI_USER -- yet > > >> this signal is definitely not the result of kill(2), tgkill(2) etc. > > >> and si_uid and si_pid make limited sense whereas we do want to > > >> yield a value for si_addr (which doesn't exist for SI_USER). > > > > > > [...] > > > > > >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > > >> index e7226c4..9040038 100644 > > >> --- a/arch/arm64/kernel/fpsimd.c > > >> +++ b/arch/arm64/kernel/fpsimd.c > > >> @@ -39,6 +39,7 @@ > > >> #include <linux/slab.h> > > >> #include <linux/sysctl.h> > > >> > > >> +#include <asm/esr.h> > > >> #include <asm/fpsimd.h> > > >> #include <asm/cputype.h> > > >> #include <asm/simd.h> > > >> @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > > >> asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > > >> { > > >> siginfo_t info; > > >> - unsigned int si_code = FPE_FIXME; > > >> - > > >> - if (esr & FPEXC_IOF) > > >> - si_code = FPE_FLTINV; > > >> - else if (esr & FPEXC_DZF) > > >> - si_code = FPE_FLTDIV; > > >> - else if (esr & FPEXC_OFF) > > >> - si_code = FPE_FLTOVF; > > >> - else if (esr & FPEXC_UFF) > > >> - si_code = FPE_FLTUND; > > >> - else if (esr & FPEXC_IXF) > > >> - si_code = FPE_FLTRES; > > >> + unsigned int si_code = FPE_FLTUNK; > > > > > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > > > resolved. > > > > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > > branch? So that there is a common branch with the code so we don't > > need to worry about conflicts. If so I will look at that on Monday. > > Yes please, that would be helpful actually. I can then pull that into > arm64 if you give me a stable branch or tag. Alternatively, I can define > FPE_FLTUNK locally and remove it at -rc1. OK Eric, let me know if you need me to rebase anything for the first 2 patches. Cheers ---Dave ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-15 21:13 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-15 21:13 UTC (permalink / raw) To: Dave Martin Cc: linux-arch, linux-api, Will Deacon, James Morse, Catalin Marinas, linux-arm-kernel Dave Martin <Dave.Martin@arm.com> writes: > On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: >> Hi Eric, >> >> On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: >> > Will Deacon <will.deacon@arm.com> writes: >> > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is >> > > resolved. >> > >> > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next >> > branch? So that there is a common branch with the code so we don't >> > need to worry about conflicts. If so I will look at that on Monday. >> >> Yes please, that would be helpful actually. I can then pull that into >> arm64 if you give me a stable branch or tag. Alternatively, I can define >> FPE_FLTUNK locally and remove it at -rc1. > > OK Eric, let me know if you need me to rebase anything for the first 2 > patches. The first patch is now in my siginfo-next tree and avialable at: git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git siginfo-next HEAD: 266da65e9156d93e1126e185259a4aae68188d0e signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions I will not rebase this branch, so please fee free to merge it into other trees. Eric ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-15 21:13 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-15 21:13 UTC (permalink / raw) To: linux-arm-kernel Dave Martin <Dave.Martin@arm.com> writes: > On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: >> Hi Eric, >> >> On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: >> > Will Deacon <will.deacon@arm.com> writes: >> > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is >> > > resolved. >> > >> > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next >> > branch? So that there is a common branch with the code so we don't >> > need to worry about conflicts. If so I will look at that on Monday. >> >> Yes please, that would be helpful actually. I can then pull that into >> arm64 if you give me a stable branch or tag. Alternatively, I can define >> FPE_FLTUNK locally and remove it at -rc1. > > OK Eric, let me know if you need me to rebase anything for the first 2 > patches. The first patch is now in my siginfo-next tree and avialable at: git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git siginfo-next HEAD: 266da65e9156d93e1126e185259a4aae68188d0e signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions I will not rebase this branch, so please fee free to merge it into other trees. Eric ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-15 21:13 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-15 21:13 UTC (permalink / raw) To: Dave Martin Cc: Will Deacon, linux-arm-kernel, linux-arch, linux-api, Catalin Marinas, James Morse Dave Martin <Dave.Martin@arm.com> writes: > On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: >> Hi Eric, >> >> On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: >> > Will Deacon <will.deacon@arm.com> writes: >> > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is >> > > resolved. >> > >> > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next >> > branch? So that there is a common branch with the code so we don't >> > need to worry about conflicts. If so I will look at that on Monday. >> >> Yes please, that would be helpful actually. I can then pull that into >> arm64 if you give me a stable branch or tag. Alternatively, I can define >> FPE_FLTUNK locally and remove it at -rc1. > > OK Eric, let me know if you need me to rebase anything for the first 2 > patches. The first patch is now in my siginfo-next tree and avialable at: git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git siginfo-next HEAD: 266da65e9156d93e1126e185259a4aae68188d0e signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions I will not rebase this branch, so please fee free to merge it into other trees. Eric ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-20 10:04 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-20 10:04 UTC (permalink / raw) To: Eric W. Biederman Cc: linux-arch, linux-api, James Morse, Catalin Marinas, Dave Martin, linux-arm-kernel On Thu, Mar 15, 2018 at 04:13:02PM -0500, Eric W. Biederman wrote: > Dave Martin <Dave.Martin@arm.com> writes: > > > On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: > >> Hi Eric, > >> > >> On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > >> > Will Deacon <will.deacon@arm.com> writes: > >> > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > >> > > resolved. > >> > > >> > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > >> > branch? So that there is a common branch with the code so we don't > >> > need to worry about conflicts. If so I will look at that on Monday. > >> > >> Yes please, that would be helpful actually. I can then pull that into > >> arm64 if you give me a stable branch or tag. Alternatively, I can define > >> FPE_FLTUNK locally and remove it at -rc1. > > > > OK Eric, let me know if you need me to rebase anything for the first 2 > > patches. > > The first patch is now in my siginfo-next tree and avialable at: > > git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git siginfo-next > HEAD: 266da65e9156d93e1126e185259a4aae68188d0e signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions > > I will not rebase this branch, so please fee free to merge it into other > trees. Thanks, Eric. I've pulled this into the arm64 for-next/core branch and I'll push it out later on. Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-20 10:04 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-20 10:04 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 15, 2018 at 04:13:02PM -0500, Eric W. Biederman wrote: > Dave Martin <Dave.Martin@arm.com> writes: > > > On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: > >> Hi Eric, > >> > >> On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > >> > Will Deacon <will.deacon@arm.com> writes: > >> > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > >> > > resolved. > >> > > >> > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > >> > branch? So that there is a common branch with the code so we don't > >> > need to worry about conflicts. If so I will look at that on Monday. > >> > >> Yes please, that would be helpful actually. I can then pull that into > >> arm64 if you give me a stable branch or tag. Alternatively, I can define > >> FPE_FLTUNK locally and remove it at -rc1. > > > > OK Eric, let me know if you need me to rebase anything for the first 2 > > patches. > > The first patch is now in my siginfo-next tree and avialable at: > > git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git siginfo-next > HEAD: 266da65e9156d93e1126e185259a4aae68188d0e signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions > > I will not rebase this branch, so please fee free to merge it into other > trees. Thanks, Eric. I've pulled this into the arm64 for-next/core branch and I'll push it out later on. Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-20 10:04 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-20 10:04 UTC (permalink / raw) To: Eric W. Biederman Cc: Dave Martin, linux-arm-kernel, linux-arch, linux-api, Catalin Marinas, James Morse On Thu, Mar 15, 2018 at 04:13:02PM -0500, Eric W. Biederman wrote: > Dave Martin <Dave.Martin@arm.com> writes: > > > On Fri, Mar 09, 2018 at 01:10:18PM +0000, Will Deacon wrote: > >> Hi Eric, > >> > >> On Thu, Mar 08, 2018 at 04:40:12PM -0600, Eric W. Biederman wrote: > >> > Will Deacon <will.deacon@arm.com> writes: > >> > > Happy to take this patch once the dependency on FPE_FLTUNK in core code is > >> > > resolved. > >> > > >> > Would it help for me to take the FPE_FLTUNK patch into my siginfo-next > >> > branch? So that there is a common branch with the code so we don't > >> > need to worry about conflicts. If so I will look at that on Monday. > >> > >> Yes please, that would be helpful actually. I can then pull that into > >> arm64 if you give me a stable branch or tag. Alternatively, I can define > >> FPE_FLTUNK locally and remove it at -rc1. > > > > OK Eric, let me know if you need me to rebase anything for the first 2 > > patches. > > The first patch is now in my siginfo-next tree and avialable at: > > git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git siginfo-next > HEAD: 266da65e9156d93e1126e185259a4aae68188d0e signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions > > I will not rebase this branch, so please fee free to merge it into other > trees. Thanks, Eric. I've pulled this into the arm64 for-next/core branch and I'll push it out later on. Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 22:37 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:37 UTC (permalink / raw) To: Dave Martin Cc: linux-arch, linux-api, Will Deacon, James Morse, Catalin Marinas, linux-arm-kernel Dave Martin <Dave.Martin@arm.com> writes: > Currently a SIGFPE delivered in response to a floating-point > exception trap may have si_code set to 0 on arm64. As reported by > Eric, this is a bad idea since this is the value of SI_USER -- yet > this signal is definitely not the result of kill(2), tgkill(2) etc. > and si_uid and si_pid make limited sense whereas we do want to > yield a value for si_addr (which doesn't exist for SI_USER). > > It's not entirely clear whether the architecure permits a > "spurious" fp exception trap where none of the exception flag bits > in ESR_ELx is set. (IMHO the architectural intent is to forbid > this.) However, it does permit those bits to contain garbage if > the TFV bit in ESR_ELx is 0. That case isn't currently handled at > all and may result in si_code == 0 or si_code containing a FPE_FLT* > constant corresponding to an exception that did not in fact happen. > > There is nothing sensible we can return for si_code in such cases, > but SI_USER is certainly not appropriate and will lead to violation > of legitimate userspace assumptions. > > This patch allocates a new si_code value FPE_UNKNOWN that at least > does not conflict with any existing SI_* or FPE_* code, and yields > this in si_code for undiagnosable cases. This is probably the best > simplicity/incorrectness tradeoff achieveable without relying on > implementation-dependent features or adding a lot of code. In any > case, there appears to be no perfect solution possible that would > justify a lot of effort here. > > Yielding FPE_UNKNOWN when some well-defined fp exception caused the > trap is a violation of POSIX, but this is forced by the > architecture. We have no realistic prospect of yielding the > correct code in such cases. At present I am not aware of any ARMv8 > implementation that supports trapped floating-point exceptions in > any case. > > The new code may be applicable to other architectures for similar > reasons. > > No attempt is made to provide ESR_ELx to userspace in the signal > frame, since architectural limitations mean that it is unlikely to > provide much diagnostic value, doesn't benefit existing software > and would create ABI with no proven purpose. The existing > mechanism for passing it also has problems of its own which may > result in the wrong value being passed to userspace due to > interaction with mm faults. The implied rework does not appear > justified. > > Reported-by: Eric W. Biederman <ebiederm@xmission.com> > Signed-off-by: Dave Martin <Dave.Martin@arm.com> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > arch/arm64/include/asm/esr.h | 9 +++++++++ > arch/arm64/include/uapi/asm/siginfo.h | 7 ------- > arch/arm64/kernel/fpsimd.c | 27 +++++++++++++++------------ > 3 files changed, 24 insertions(+), 19 deletions(-) > > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h > index 803443d..ce70c3f 100644 > --- a/arch/arm64/include/asm/esr.h > +++ b/arch/arm64/include/asm/esr.h > @@ -240,6 +240,15 @@ > (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ > ESR_ELx_SYS64_ISS_OP2_SHIFT)) > > +/* > + * ISS field definitions for floating-point exception traps > + * (FP_EXC_32/FP_EXC_64). > + * > + * (The FPEXC_* constants are used instead for common bits.) > + */ > + > +#define ESR_ELx_FP_EXC_TFV (UL(1) << 23) > + > #ifndef __ASSEMBLY__ > #include <asm/types.h> > > diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h > index 9b4d912..157e6a8 100644 > --- a/arch/arm64/include/uapi/asm/siginfo.h > +++ b/arch/arm64/include/uapi/asm/siginfo.h > @@ -22,13 +22,6 @@ > #include <asm-generic/siginfo.h> > > /* > - * SIGFPE si_codes > - */ > -#ifdef __KERNEL__ > -#define FPE_FIXME 0 /* Broken dup of SI_USER */ > -#endif /* __KERNEL__ */ > - > -/* > * SIGBUS si_codes > */ > #ifdef __KERNEL__ > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index e7226c4..9040038 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -39,6 +39,7 @@ > #include <linux/slab.h> > #include <linux/sysctl.h> > > +#include <asm/esr.h> > #include <asm/fpsimd.h> > #include <asm/cputype.h> > #include <asm/simd.h> > @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > { > siginfo_t info; > - unsigned int si_code = FPE_FIXME; > - > - if (esr & FPEXC_IOF) > - si_code = FPE_FLTINV; > - else if (esr & FPEXC_DZF) > - si_code = FPE_FLTDIV; > - else if (esr & FPEXC_OFF) > - si_code = FPE_FLTOVF; > - else if (esr & FPEXC_UFF) > - si_code = FPE_FLTUND; > - else if (esr & FPEXC_IXF) > - si_code = FPE_FLTRES; > + unsigned int si_code = FPE_FLTUNK; > + > + if (esr & ESR_ELx_FP_EXC_TFV) { > + if (esr & FPEXC_IOF) > + si_code = FPE_FLTINV; > + else if (esr & FPEXC_DZF) > + si_code = FPE_FLTDIV; > + else if (esr & FPEXC_OFF) > + si_code = FPE_FLTOVF; > + else if (esr & FPEXC_UFF) > + si_code = FPE_FLTUND; > + else if (esr & FPEXC_IXF) > + si_code = FPE_FLTRES; > + } > > memset(&info, 0, sizeof(info)); > info.si_signo = SIGFPE; ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 22:37 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:37 UTC (permalink / raw) To: linux-arm-kernel Dave Martin <Dave.Martin@arm.com> writes: > Currently a SIGFPE delivered in response to a floating-point > exception trap may have si_code set to 0 on arm64. As reported by > Eric, this is a bad idea since this is the value of SI_USER -- yet > this signal is definitely not the result of kill(2), tgkill(2) etc. > and si_uid and si_pid make limited sense whereas we do want to > yield a value for si_addr (which doesn't exist for SI_USER). > > It's not entirely clear whether the architecure permits a > "spurious" fp exception trap where none of the exception flag bits > in ESR_ELx is set. (IMHO the architectural intent is to forbid > this.) However, it does permit those bits to contain garbage if > the TFV bit in ESR_ELx is 0. That case isn't currently handled at > all and may result in si_code == 0 or si_code containing a FPE_FLT* > constant corresponding to an exception that did not in fact happen. > > There is nothing sensible we can return for si_code in such cases, > but SI_USER is certainly not appropriate and will lead to violation > of legitimate userspace assumptions. > > This patch allocates a new si_code value FPE_UNKNOWN that at least > does not conflict with any existing SI_* or FPE_* code, and yields > this in si_code for undiagnosable cases. This is probably the best > simplicity/incorrectness tradeoff achieveable without relying on > implementation-dependent features or adding a lot of code. In any > case, there appears to be no perfect solution possible that would > justify a lot of effort here. > > Yielding FPE_UNKNOWN when some well-defined fp exception caused the > trap is a violation of POSIX, but this is forced by the > architecture. We have no realistic prospect of yielding the > correct code in such cases. At present I am not aware of any ARMv8 > implementation that supports trapped floating-point exceptions in > any case. > > The new code may be applicable to other architectures for similar > reasons. > > No attempt is made to provide ESR_ELx to userspace in the signal > frame, since architectural limitations mean that it is unlikely to > provide much diagnostic value, doesn't benefit existing software > and would create ABI with no proven purpose. The existing > mechanism for passing it also has problems of its own which may > result in the wrong value being passed to userspace due to > interaction with mm faults. The implied rework does not appear > justified. > > Reported-by: Eric W. Biederman <ebiederm@xmission.com> > Signed-off-by: Dave Martin <Dave.Martin@arm.com> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > arch/arm64/include/asm/esr.h | 9 +++++++++ > arch/arm64/include/uapi/asm/siginfo.h | 7 ------- > arch/arm64/kernel/fpsimd.c | 27 +++++++++++++++------------ > 3 files changed, 24 insertions(+), 19 deletions(-) > > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h > index 803443d..ce70c3f 100644 > --- a/arch/arm64/include/asm/esr.h > +++ b/arch/arm64/include/asm/esr.h > @@ -240,6 +240,15 @@ > (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ > ESR_ELx_SYS64_ISS_OP2_SHIFT)) > > +/* > + * ISS field definitions for floating-point exception traps > + * (FP_EXC_32/FP_EXC_64). > + * > + * (The FPEXC_* constants are used instead for common bits.) > + */ > + > +#define ESR_ELx_FP_EXC_TFV (UL(1) << 23) > + > #ifndef __ASSEMBLY__ > #include <asm/types.h> > > diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h > index 9b4d912..157e6a8 100644 > --- a/arch/arm64/include/uapi/asm/siginfo.h > +++ b/arch/arm64/include/uapi/asm/siginfo.h > @@ -22,13 +22,6 @@ > #include <asm-generic/siginfo.h> > > /* > - * SIGFPE si_codes > - */ > -#ifdef __KERNEL__ > -#define FPE_FIXME 0 /* Broken dup of SI_USER */ > -#endif /* __KERNEL__ */ > - > -/* > * SIGBUS si_codes > */ > #ifdef __KERNEL__ > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index e7226c4..9040038 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -39,6 +39,7 @@ > #include <linux/slab.h> > #include <linux/sysctl.h> > > +#include <asm/esr.h> > #include <asm/fpsimd.h> > #include <asm/cputype.h> > #include <asm/simd.h> > @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > { > siginfo_t info; > - unsigned int si_code = FPE_FIXME; > - > - if (esr & FPEXC_IOF) > - si_code = FPE_FLTINV; > - else if (esr & FPEXC_DZF) > - si_code = FPE_FLTDIV; > - else if (esr & FPEXC_OFF) > - si_code = FPE_FLTOVF; > - else if (esr & FPEXC_UFF) > - si_code = FPE_FLTUND; > - else if (esr & FPEXC_IXF) > - si_code = FPE_FLTRES; > + unsigned int si_code = FPE_FLTUNK; > + > + if (esr & ESR_ELx_FP_EXC_TFV) { > + if (esr & FPEXC_IOF) > + si_code = FPE_FLTINV; > + else if (esr & FPEXC_DZF) > + si_code = FPE_FLTDIV; > + else if (esr & FPEXC_OFF) > + si_code = FPE_FLTOVF; > + else if (esr & FPEXC_UFF) > + si_code = FPE_FLTUND; > + else if (esr & FPEXC_IXF) > + si_code = FPE_FLTRES; > + } > > memset(&info, 0, sizeof(info)); > info.si_signo = SIGFPE; ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE @ 2018-03-08 22:37 ` Eric W. Biederman 0 siblings, 0 replies; 41+ messages in thread From: Eric W. Biederman @ 2018-03-08 22:37 UTC (permalink / raw) To: Dave Martin Cc: linux-arm-kernel, linux-arch, linux-api, Will Deacon, Catalin Marinas, James Morse Dave Martin <Dave.Martin@arm.com> writes: > Currently a SIGFPE delivered in response to a floating-point > exception trap may have si_code set to 0 on arm64. As reported by > Eric, this is a bad idea since this is the value of SI_USER -- yet > this signal is definitely not the result of kill(2), tgkill(2) etc. > and si_uid and si_pid make limited sense whereas we do want to > yield a value for si_addr (which doesn't exist for SI_USER). > > It's not entirely clear whether the architecure permits a > "spurious" fp exception trap where none of the exception flag bits > in ESR_ELx is set. (IMHO the architectural intent is to forbid > this.) However, it does permit those bits to contain garbage if > the TFV bit in ESR_ELx is 0. That case isn't currently handled at > all and may result in si_code == 0 or si_code containing a FPE_FLT* > constant corresponding to an exception that did not in fact happen. > > There is nothing sensible we can return for si_code in such cases, > but SI_USER is certainly not appropriate and will lead to violation > of legitimate userspace assumptions. > > This patch allocates a new si_code value FPE_UNKNOWN that at least > does not conflict with any existing SI_* or FPE_* code, and yields > this in si_code for undiagnosable cases. This is probably the best > simplicity/incorrectness tradeoff achieveable without relying on > implementation-dependent features or adding a lot of code. In any > case, there appears to be no perfect solution possible that would > justify a lot of effort here. > > Yielding FPE_UNKNOWN when some well-defined fp exception caused the > trap is a violation of POSIX, but this is forced by the > architecture. We have no realistic prospect of yielding the > correct code in such cases. At present I am not aware of any ARMv8 > implementation that supports trapped floating-point exceptions in > any case. > > The new code may be applicable to other architectures for similar > reasons. > > No attempt is made to provide ESR_ELx to userspace in the signal > frame, since architectural limitations mean that it is unlikely to > provide much diagnostic value, doesn't benefit existing software > and would create ABI with no proven purpose. The existing > mechanism for passing it also has problems of its own which may > result in the wrong value being passed to userspace due to > interaction with mm faults. The implied rework does not appear > justified. > > Reported-by: Eric W. Biederman <ebiederm@xmission.com> > Signed-off-by: Dave Martin <Dave.Martin@arm.com> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > arch/arm64/include/asm/esr.h | 9 +++++++++ > arch/arm64/include/uapi/asm/siginfo.h | 7 ------- > arch/arm64/kernel/fpsimd.c | 27 +++++++++++++++------------ > 3 files changed, 24 insertions(+), 19 deletions(-) > > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h > index 803443d..ce70c3f 100644 > --- a/arch/arm64/include/asm/esr.h > +++ b/arch/arm64/include/asm/esr.h > @@ -240,6 +240,15 @@ > (((e) & ESR_ELx_SYS64_ISS_OP2_MASK) >> \ > ESR_ELx_SYS64_ISS_OP2_SHIFT)) > > +/* > + * ISS field definitions for floating-point exception traps > + * (FP_EXC_32/FP_EXC_64). > + * > + * (The FPEXC_* constants are used instead for common bits.) > + */ > + > +#define ESR_ELx_FP_EXC_TFV (UL(1) << 23) > + > #ifndef __ASSEMBLY__ > #include <asm/types.h> > > diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h > index 9b4d912..157e6a8 100644 > --- a/arch/arm64/include/uapi/asm/siginfo.h > +++ b/arch/arm64/include/uapi/asm/siginfo.h > @@ -22,13 +22,6 @@ > #include <asm-generic/siginfo.h> > > /* > - * SIGFPE si_codes > - */ > -#ifdef __KERNEL__ > -#define FPE_FIXME 0 /* Broken dup of SI_USER */ > -#endif /* __KERNEL__ */ > - > -/* > * SIGBUS si_codes > */ > #ifdef __KERNEL__ > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index e7226c4..9040038 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -39,6 +39,7 @@ > #include <linux/slab.h> > #include <linux/sysctl.h> > > +#include <asm/esr.h> > #include <asm/fpsimd.h> > #include <asm/cputype.h> > #include <asm/simd.h> > @@ -867,18 +868,20 @@ asmlinkage void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs) > asmlinkage void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs) > { > siginfo_t info; > - unsigned int si_code = FPE_FIXME; > - > - if (esr & FPEXC_IOF) > - si_code = FPE_FLTINV; > - else if (esr & FPEXC_DZF) > - si_code = FPE_FLTDIV; > - else if (esr & FPEXC_OFF) > - si_code = FPE_FLTOVF; > - else if (esr & FPEXC_UFF) > - si_code = FPE_FLTUND; > - else if (esr & FPEXC_IXF) > - si_code = FPE_FLTRES; > + unsigned int si_code = FPE_FLTUNK; > + > + if (esr & ESR_ELx_FP_EXC_TFV) { > + if (esr & FPEXC_IOF) > + si_code = FPE_FLTINV; > + else if (esr & FPEXC_DZF) > + si_code = FPE_FLTDIV; > + else if (esr & FPEXC_OFF) > + si_code = FPE_FLTOVF; > + else if (esr & FPEXC_UFF) > + si_code = FPE_FLTUND; > + else if (esr & FPEXC_IXF) > + si_code = FPE_FLTRES; > + } > > memset(&info, 0, sizeof(info)); > info.si_signo = SIGFPE; ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Will Deacon, James Morse, Eric W. Biederman, Catalin Marinas Currently, as reported by Eric, an invalid si_code value 0 is passed in many signals delivered to userspace in response to faults and other kernel errors. Typically 0 is passed when the fault is insufficiently diagnosable or when there does not appear to be any sensible alternative value to choose. This appears to violate POSIX, and is intuitively wrong for at least two reasons arising from the fact that 0 == SI_USER: 1) si_code is a union selector, and SI_USER (and si_code <= 0 in general) implies the existence of a different set of fields (siginfo._kill) from that which exists for a fault signal (siginfo._sigfault). However, the code raising the signal typically writes only the _sigfault fields, and the _kill fields make no sense in this case. Thus when userspace sees si_code == 0 (SI_USER) it may legitimately inspect fields in the inactive union member _kill and obtain garbage as a result. There appears to be software in the wild relying on this, albeit generally only for printing diagnostic messages. 2) Software that wants to be robust against spurious signals may discard signals where si_code == SI_USER (or <= 0), or may filter such signals based on the si_uid and si_pid fields of siginfo._sigkill. In the case of fault signals, this means that important (and usually fatal) error conditions may be silently ignored. In practice, many of the faults for which arm64 passes si_code == 0 are undiagnosable conditions such as exceptions with syndrome values in ESR_ELx to which the architecture does not yet assign any meaning, or conditions indicative of a bug or error in the kernel or system and thus that are unrecoverable and should never occur in normal operation. The approach taken in this patch is to translate all such undiagnosable or "impossible" synchronous fault conditions to SIGKILL, since these are at least probably localisable to a single process. Some of these conditions should really result in a kernel panic, but due to the lack of diagnostic information it is difficult to be certain: this patch does not add any calls to panic(), but this could change later if justified. Although si_code will not reach userspace in the case of SIGKILL, it is still desirable to pass a nonzero value so that the common siginfo handling code can detect incorrect use of si_code == 0 without false positives. In this case the si_code dependent siginfo fields will not be correctly initialised, but since they are not passed to userspace I deem this not to matter. A few faults can reasonably occur in realistic userspace scenarios, and _should_ raise a regular, handleable (but perhaps not ignorable/blockable) signal: for these, this patch attempts to choose a suitable standard si_code value for the raised signal in each case instead of 0. arm64 was the only arch to define a BUS_FIXME code, so after this patch nobody defines it. This patch therefore also removes the relevant code from siginfo_layout(). Cc: James Morse <james.morse@arm.com> Reported-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- Changes since RFC v2: * Squashed in revert of the appropriate bits of Eric's BUS_/TRAP_FIXME stuff. * Propagated the fault_info[] sig and code vaules through do_sea(), which previously was ignoring them. --- arch/arm64/include/uapi/asm/siginfo.h | 14 ---- arch/arm64/kernel/fpsimd.c | 4 +- arch/arm64/mm/fault.c | 116 +++++++++++++++++----------------- kernel/signal.c | 4 -- 4 files changed, 60 insertions(+), 78 deletions(-) diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 157e6a8..574d12f 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -21,18 +21,4 @@ #include <asm-generic/siginfo.h> -/* - * SIGBUS si_codes - */ -#ifdef __KERNEL__ -#define BUS_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - -/* - * SIGTRAP si_codes - */ -#ifdef __KERNEL__ -#define TRAP_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - #endif diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 9040038..dc93aa4 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -286,8 +286,8 @@ static void task_fpsimd_save(void) * re-enter user with corrupt state. * There's no way to recover, so kill it: */ - force_signal_inject( - SIGKILL, 0, current_pt_regs(), 0); + force_signal_inject(SIGKILL, SI_KERNEL, + current_pt_regs(), 0); return; } diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index bff1155..c7c85c3 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -600,9 +600,9 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) nmi_exit(); } - info.si_signo = SIGBUS; + info.si_signo = inf->sig; info.si_errno = 0; - info.si_code = BUS_FIXME; + info.si_code = inf->code; if (esr & ESR_ELx_FnV) info.si_addr = NULL; else @@ -613,70 +613,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) } static const struct fault_info fault_info[] = { - { do_bad, SIGBUS, BUS_FIXME, "ttbr address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 1 address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 2 address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 3 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "ttbr address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 1 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 2 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 3 address size fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 0 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 1 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 2 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 3 translation fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 8" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 8" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 access flag fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 access flag fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 access flag fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 12" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 12" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 permission fault" }, - { do_sea, SIGBUS, BUS_FIXME, "synchronous external abort" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 17" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 18" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 19" }, - { do_sea, SIGBUS, BUS_FIXME, "level 0 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 1 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 2 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 3 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "synchronous parity or ECC error" }, // Reserved when RAS is implemented - { do_bad, SIGBUS, BUS_FIXME, "unknown 25" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 26" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 27" }, - { do_sea, SIGBUS, BUS_FIXME, "level 0 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 1 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 2 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 3 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_bad, SIGBUS, BUS_FIXME, "unknown 32" }, + { do_sea, SIGBUS, BUS_OBJERR, "synchronous external abort" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 17" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 18" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 19" }, + { do_sea, SIGKILL, SI_KERNEL, "level 0 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 1 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 2 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 3 (translation table walk)" }, + { do_sea, SIGBUS, BUS_OBJERR, "synchronous parity or ECC error" }, // Reserved when RAS is implemented + { do_bad, SIGKILL, SI_KERNEL, "unknown 25" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 26" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 27" }, + { do_sea, SIGKILL, SI_KERNEL, "level 0 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 1 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 2 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 3 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_bad, SIGKILL, SI_KERNEL, "unknown 32" }, { do_alignment_fault, SIGBUS, BUS_ADRALN, "alignment fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 34" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 35" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 36" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 37" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 38" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 39" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 40" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 41" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 42" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 43" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 44" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 45" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 46" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 47" }, - { do_bad, SIGBUS, BUS_FIXME, "TLB conflict abort" }, - { do_bad, SIGBUS, BUS_FIXME, "Unsupported atomic hardware update fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 50" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 51" }, - { do_bad, SIGBUS, BUS_FIXME, "implementation fault (lockdown abort)" }, - { do_bad, SIGBUS, BUS_FIXME, "implementation fault (unsupported exclusive)" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 54" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 55" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 56" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 57" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 58" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 59" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 60" }, - { do_bad, SIGBUS, BUS_FIXME, "section domain fault" }, - { do_bad, SIGBUS, BUS_FIXME, "page domain fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 63" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 34" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 35" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 36" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 37" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 38" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 39" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 40" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 41" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 42" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 43" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 44" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 45" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 46" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 47" }, + { do_bad, SIGKILL, SI_KERNEL, "TLB conflict abort" }, + { do_bad, SIGKILL, SI_KERNEL, "Unsupported atomic hardware update fault" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 50" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 51" }, + { do_bad, SIGKILL, SI_KERNEL, "implementation fault (lockdown abort)" }, + { do_bad, SIGBUS, BUS_OBJERR, "implementation fault (unsupported exclusive)" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 54" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 55" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 56" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 57" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 58" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 59" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 60" }, + { do_bad, SIGKILL, SI_KERNEL, "section domain fault" }, + { do_bad, SIGKILL, SI_KERNEL, "page domain fault" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 63" }, }; int handle_guest_sea(phys_addr_t addr, unsigned int esr) @@ -774,11 +774,11 @@ static struct fault_info __refdata debug_fault_info[] = { { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware breakpoint" }, { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware single-step" }, { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware watchpoint" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 3" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 3" }, { do_bad, SIGTRAP, TRAP_BRKPT, "aarch32 BKPT" }, - { do_bad, SIGTRAP, TRAP_FIXME, "aarch32 vector catch" }, + { do_bad, SIGKILL, SI_KERNEL, "aarch32 vector catch" }, { early_brk64, SIGTRAP, TRAP_BRKPT, "aarch64 BRK" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 7" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 7" }, }; void __init hook_debug_fault_code(int nr, diff --git a/kernel/signal.c b/kernel/signal.c index c6e4c83d..049a482 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2844,10 +2844,6 @@ enum siginfo_layout siginfo_layout(int sig, int si_code) if ((sig == SIGFPE) && (si_code == FPE_FIXME)) layout = SIL_FAULT; #endif -#ifdef BUS_FIXME - if ((sig == SIGBUS) && (si_code == BUS_FIXME)) - layout = SIL_FAULT; -#endif } return layout; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Currently, as reported by Eric, an invalid si_code value 0 is passed in many signals delivered to userspace in response to faults and other kernel errors. Typically 0 is passed when the fault is insufficiently diagnosable or when there does not appear to be any sensible alternative value to choose. This appears to violate POSIX, and is intuitively wrong for at least two reasons arising from the fact that 0 == SI_USER: 1) si_code is a union selector, and SI_USER (and si_code <= 0 in general) implies the existence of a different set of fields (siginfo._kill) from that which exists for a fault signal (siginfo._sigfault). However, the code raising the signal typically writes only the _sigfault fields, and the _kill fields make no sense in this case. Thus when userspace sees si_code == 0 (SI_USER) it may legitimately inspect fields in the inactive union member _kill and obtain garbage as a result. There appears to be software in the wild relying on this, albeit generally only for printing diagnostic messages. 2) Software that wants to be robust against spurious signals may discard signals where si_code == SI_USER (or <= 0), or may filter such signals based on the si_uid and si_pid fields of siginfo._sigkill. In the case of fault signals, this means that important (and usually fatal) error conditions may be silently ignored. In practice, many of the faults for which arm64 passes si_code == 0 are undiagnosable conditions such as exceptions with syndrome values in ESR_ELx to which the architecture does not yet assign any meaning, or conditions indicative of a bug or error in the kernel or system and thus that are unrecoverable and should never occur in normal operation. The approach taken in this patch is to translate all such undiagnosable or "impossible" synchronous fault conditions to SIGKILL, since these are at least probably localisable to a single process. Some of these conditions should really result in a kernel panic, but due to the lack of diagnostic information it is difficult to be certain: this patch does not add any calls to panic(), but this could change later if justified. Although si_code will not reach userspace in the case of SIGKILL, it is still desirable to pass a nonzero value so that the common siginfo handling code can detect incorrect use of si_code == 0 without false positives. In this case the si_code dependent siginfo fields will not be correctly initialised, but since they are not passed to userspace I deem this not to matter. A few faults can reasonably occur in realistic userspace scenarios, and _should_ raise a regular, handleable (but perhaps not ignorable/blockable) signal: for these, this patch attempts to choose a suitable standard si_code value for the raised signal in each case instead of 0. arm64 was the only arch to define a BUS_FIXME code, so after this patch nobody defines it. This patch therefore also removes the relevant code from siginfo_layout(). Cc: James Morse <james.morse@arm.com> Reported-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- Changes since RFC v2: * Squashed in revert of the appropriate bits of Eric's BUS_/TRAP_FIXME stuff. * Propagated the fault_info[] sig and code vaules through do_sea(), which previously was ignoring them. --- arch/arm64/include/uapi/asm/siginfo.h | 14 ---- arch/arm64/kernel/fpsimd.c | 4 +- arch/arm64/mm/fault.c | 116 +++++++++++++++++----------------- kernel/signal.c | 4 -- 4 files changed, 60 insertions(+), 78 deletions(-) diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 157e6a8..574d12f 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -21,18 +21,4 @@ #include <asm-generic/siginfo.h> -/* - * SIGBUS si_codes - */ -#ifdef __KERNEL__ -#define BUS_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - -/* - * SIGTRAP si_codes - */ -#ifdef __KERNEL__ -#define TRAP_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - #endif diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 9040038..dc93aa4 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -286,8 +286,8 @@ static void task_fpsimd_save(void) * re-enter user with corrupt state. * There's no way to recover, so kill it: */ - force_signal_inject( - SIGKILL, 0, current_pt_regs(), 0); + force_signal_inject(SIGKILL, SI_KERNEL, + current_pt_regs(), 0); return; } diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index bff1155..c7c85c3 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -600,9 +600,9 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) nmi_exit(); } - info.si_signo = SIGBUS; + info.si_signo = inf->sig; info.si_errno = 0; - info.si_code = BUS_FIXME; + info.si_code = inf->code; if (esr & ESR_ELx_FnV) info.si_addr = NULL; else @@ -613,70 +613,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) } static const struct fault_info fault_info[] = { - { do_bad, SIGBUS, BUS_FIXME, "ttbr address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 1 address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 2 address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 3 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "ttbr address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 1 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 2 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 3 address size fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 0 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 1 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 2 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 3 translation fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 8" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 8" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 access flag fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 access flag fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 access flag fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 12" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 12" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 permission fault" }, - { do_sea, SIGBUS, BUS_FIXME, "synchronous external abort" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 17" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 18" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 19" }, - { do_sea, SIGBUS, BUS_FIXME, "level 0 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 1 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 2 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 3 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "synchronous parity or ECC error" }, // Reserved when RAS is implemented - { do_bad, SIGBUS, BUS_FIXME, "unknown 25" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 26" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 27" }, - { do_sea, SIGBUS, BUS_FIXME, "level 0 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 1 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 2 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 3 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_bad, SIGBUS, BUS_FIXME, "unknown 32" }, + { do_sea, SIGBUS, BUS_OBJERR, "synchronous external abort" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 17" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 18" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 19" }, + { do_sea, SIGKILL, SI_KERNEL, "level 0 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 1 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 2 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 3 (translation table walk)" }, + { do_sea, SIGBUS, BUS_OBJERR, "synchronous parity or ECC error" }, // Reserved when RAS is implemented + { do_bad, SIGKILL, SI_KERNEL, "unknown 25" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 26" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 27" }, + { do_sea, SIGKILL, SI_KERNEL, "level 0 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 1 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 2 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 3 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_bad, SIGKILL, SI_KERNEL, "unknown 32" }, { do_alignment_fault, SIGBUS, BUS_ADRALN, "alignment fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 34" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 35" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 36" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 37" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 38" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 39" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 40" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 41" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 42" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 43" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 44" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 45" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 46" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 47" }, - { do_bad, SIGBUS, BUS_FIXME, "TLB conflict abort" }, - { do_bad, SIGBUS, BUS_FIXME, "Unsupported atomic hardware update fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 50" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 51" }, - { do_bad, SIGBUS, BUS_FIXME, "implementation fault (lockdown abort)" }, - { do_bad, SIGBUS, BUS_FIXME, "implementation fault (unsupported exclusive)" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 54" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 55" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 56" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 57" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 58" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 59" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 60" }, - { do_bad, SIGBUS, BUS_FIXME, "section domain fault" }, - { do_bad, SIGBUS, BUS_FIXME, "page domain fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 63" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 34" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 35" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 36" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 37" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 38" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 39" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 40" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 41" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 42" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 43" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 44" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 45" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 46" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 47" }, + { do_bad, SIGKILL, SI_KERNEL, "TLB conflict abort" }, + { do_bad, SIGKILL, SI_KERNEL, "Unsupported atomic hardware update fault" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 50" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 51" }, + { do_bad, SIGKILL, SI_KERNEL, "implementation fault (lockdown abort)" }, + { do_bad, SIGBUS, BUS_OBJERR, "implementation fault (unsupported exclusive)" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 54" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 55" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 56" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 57" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 58" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 59" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 60" }, + { do_bad, SIGKILL, SI_KERNEL, "section domain fault" }, + { do_bad, SIGKILL, SI_KERNEL, "page domain fault" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 63" }, }; int handle_guest_sea(phys_addr_t addr, unsigned int esr) @@ -774,11 +774,11 @@ static struct fault_info __refdata debug_fault_info[] = { { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware breakpoint" }, { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware single-step" }, { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware watchpoint" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 3" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 3" }, { do_bad, SIGTRAP, TRAP_BRKPT, "aarch32 BKPT" }, - { do_bad, SIGTRAP, TRAP_FIXME, "aarch32 vector catch" }, + { do_bad, SIGKILL, SI_KERNEL, "aarch32 vector catch" }, { early_brk64, SIGTRAP, TRAP_BRKPT, "aarch64 BRK" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 7" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 7" }, }; void __init hook_debug_fault_code(int nr, diff --git a/kernel/signal.c b/kernel/signal.c index c6e4c83d..049a482 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2844,10 +2844,6 @@ enum siginfo_layout siginfo_layout(int sig, int si_code) if ((sig == SIGFPE) && (si_code == FPE_FIXME)) layout = SIL_FAULT; #endif -#ifdef BUS_FIXME - if ((sig == SIGBUS) && (si_code == BUS_FIXME)) - layout = SIL_FAULT; -#endif } return layout; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals @ 2018-03-01 17:44 ` Dave Martin 0 siblings, 0 replies; 41+ messages in thread From: Dave Martin @ 2018-03-01 17:44 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-arch, linux-api, Eric W. Biederman, Will Deacon, Catalin Marinas, James Morse Currently, as reported by Eric, an invalid si_code value 0 is passed in many signals delivered to userspace in response to faults and other kernel errors. Typically 0 is passed when the fault is insufficiently diagnosable or when there does not appear to be any sensible alternative value to choose. This appears to violate POSIX, and is intuitively wrong for at least two reasons arising from the fact that 0 == SI_USER: 1) si_code is a union selector, and SI_USER (and si_code <= 0 in general) implies the existence of a different set of fields (siginfo._kill) from that which exists for a fault signal (siginfo._sigfault). However, the code raising the signal typically writes only the _sigfault fields, and the _kill fields make no sense in this case. Thus when userspace sees si_code == 0 (SI_USER) it may legitimately inspect fields in the inactive union member _kill and obtain garbage as a result. There appears to be software in the wild relying on this, albeit generally only for printing diagnostic messages. 2) Software that wants to be robust against spurious signals may discard signals where si_code == SI_USER (or <= 0), or may filter such signals based on the si_uid and si_pid fields of siginfo._sigkill. In the case of fault signals, this means that important (and usually fatal) error conditions may be silently ignored. In practice, many of the faults for which arm64 passes si_code == 0 are undiagnosable conditions such as exceptions with syndrome values in ESR_ELx to which the architecture does not yet assign any meaning, or conditions indicative of a bug or error in the kernel or system and thus that are unrecoverable and should never occur in normal operation. The approach taken in this patch is to translate all such undiagnosable or "impossible" synchronous fault conditions to SIGKILL, since these are at least probably localisable to a single process. Some of these conditions should really result in a kernel panic, but due to the lack of diagnostic information it is difficult to be certain: this patch does not add any calls to panic(), but this could change later if justified. Although si_code will not reach userspace in the case of SIGKILL, it is still desirable to pass a nonzero value so that the common siginfo handling code can detect incorrect use of si_code == 0 without false positives. In this case the si_code dependent siginfo fields will not be correctly initialised, but since they are not passed to userspace I deem this not to matter. A few faults can reasonably occur in realistic userspace scenarios, and _should_ raise a regular, handleable (but perhaps not ignorable/blockable) signal: for these, this patch attempts to choose a suitable standard si_code value for the raised signal in each case instead of 0. arm64 was the only arch to define a BUS_FIXME code, so after this patch nobody defines it. This patch therefore also removes the relevant code from siginfo_layout(). Cc: James Morse <james.morse@arm.com> Reported-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- Changes since RFC v2: * Squashed in revert of the appropriate bits of Eric's BUS_/TRAP_FIXME stuff. * Propagated the fault_info[] sig and code vaules through do_sea(), which previously was ignoring them. --- arch/arm64/include/uapi/asm/siginfo.h | 14 ---- arch/arm64/kernel/fpsimd.c | 4 +- arch/arm64/mm/fault.c | 116 +++++++++++++++++----------------- kernel/signal.c | 4 -- 4 files changed, 60 insertions(+), 78 deletions(-) diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 157e6a8..574d12f 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -21,18 +21,4 @@ #include <asm-generic/siginfo.h> -/* - * SIGBUS si_codes - */ -#ifdef __KERNEL__ -#define BUS_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - -/* - * SIGTRAP si_codes - */ -#ifdef __KERNEL__ -#define TRAP_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - #endif diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 9040038..dc93aa4 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -286,8 +286,8 @@ static void task_fpsimd_save(void) * re-enter user with corrupt state. * There's no way to recover, so kill it: */ - force_signal_inject( - SIGKILL, 0, current_pt_regs(), 0); + force_signal_inject(SIGKILL, SI_KERNEL, + current_pt_regs(), 0); return; } diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index bff1155..c7c85c3 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -600,9 +600,9 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) nmi_exit(); } - info.si_signo = SIGBUS; + info.si_signo = inf->sig; info.si_errno = 0; - info.si_code = BUS_FIXME; + info.si_code = inf->code; if (esr & ESR_ELx_FnV) info.si_addr = NULL; else @@ -613,70 +613,70 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) } static const struct fault_info fault_info[] = { - { do_bad, SIGBUS, BUS_FIXME, "ttbr address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 1 address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 2 address size fault" }, - { do_bad, SIGBUS, BUS_FIXME, "level 3 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "ttbr address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 1 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 2 address size fault" }, + { do_bad, SIGKILL, SI_KERNEL, "level 3 address size fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 0 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 1 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 2 translation fault" }, { do_translation_fault, SIGSEGV, SEGV_MAPERR, "level 3 translation fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 8" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 8" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 access flag fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 access flag fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 access flag fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 12" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 12" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 permission fault" }, - { do_sea, SIGBUS, BUS_FIXME, "synchronous external abort" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 17" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 18" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 19" }, - { do_sea, SIGBUS, BUS_FIXME, "level 0 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 1 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 2 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "level 3 (translation table walk)" }, - { do_sea, SIGBUS, BUS_FIXME, "synchronous parity or ECC error" }, // Reserved when RAS is implemented - { do_bad, SIGBUS, BUS_FIXME, "unknown 25" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 26" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 27" }, - { do_sea, SIGBUS, BUS_FIXME, "level 0 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 1 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 2 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_sea, SIGBUS, BUS_FIXME, "level 3 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented - { do_bad, SIGBUS, BUS_FIXME, "unknown 32" }, + { do_sea, SIGBUS, BUS_OBJERR, "synchronous external abort" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 17" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 18" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 19" }, + { do_sea, SIGKILL, SI_KERNEL, "level 0 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 1 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 2 (translation table walk)" }, + { do_sea, SIGKILL, SI_KERNEL, "level 3 (translation table walk)" }, + { do_sea, SIGBUS, BUS_OBJERR, "synchronous parity or ECC error" }, // Reserved when RAS is implemented + { do_bad, SIGKILL, SI_KERNEL, "unknown 25" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 26" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 27" }, + { do_sea, SIGKILL, SI_KERNEL, "level 0 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 1 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 2 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_sea, SIGKILL, SI_KERNEL, "level 3 synchronous parity error (translation table walk)" }, // Reserved when RAS is implemented + { do_bad, SIGKILL, SI_KERNEL, "unknown 32" }, { do_alignment_fault, SIGBUS, BUS_ADRALN, "alignment fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 34" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 35" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 36" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 37" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 38" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 39" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 40" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 41" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 42" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 43" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 44" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 45" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 46" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 47" }, - { do_bad, SIGBUS, BUS_FIXME, "TLB conflict abort" }, - { do_bad, SIGBUS, BUS_FIXME, "Unsupported atomic hardware update fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 50" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 51" }, - { do_bad, SIGBUS, BUS_FIXME, "implementation fault (lockdown abort)" }, - { do_bad, SIGBUS, BUS_FIXME, "implementation fault (unsupported exclusive)" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 54" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 55" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 56" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 57" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 58" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 59" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 60" }, - { do_bad, SIGBUS, BUS_FIXME, "section domain fault" }, - { do_bad, SIGBUS, BUS_FIXME, "page domain fault" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 63" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 34" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 35" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 36" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 37" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 38" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 39" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 40" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 41" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 42" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 43" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 44" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 45" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 46" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 47" }, + { do_bad, SIGKILL, SI_KERNEL, "TLB conflict abort" }, + { do_bad, SIGKILL, SI_KERNEL, "Unsupported atomic hardware update fault" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 50" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 51" }, + { do_bad, SIGKILL, SI_KERNEL, "implementation fault (lockdown abort)" }, + { do_bad, SIGBUS, BUS_OBJERR, "implementation fault (unsupported exclusive)" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 54" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 55" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 56" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 57" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 58" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 59" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 60" }, + { do_bad, SIGKILL, SI_KERNEL, "section domain fault" }, + { do_bad, SIGKILL, SI_KERNEL, "page domain fault" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 63" }, }; int handle_guest_sea(phys_addr_t addr, unsigned int esr) @@ -774,11 +774,11 @@ static struct fault_info __refdata debug_fault_info[] = { { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware breakpoint" }, { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware single-step" }, { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware watchpoint" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 3" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 3" }, { do_bad, SIGTRAP, TRAP_BRKPT, "aarch32 BKPT" }, - { do_bad, SIGTRAP, TRAP_FIXME, "aarch32 vector catch" }, + { do_bad, SIGKILL, SI_KERNEL, "aarch32 vector catch" }, { early_brk64, SIGTRAP, TRAP_BRKPT, "aarch64 BRK" }, - { do_bad, SIGBUS, BUS_FIXME, "unknown 7" }, + { do_bad, SIGKILL, SI_KERNEL, "unknown 7" }, }; void __init hook_debug_fault_code(int nr, diff --git a/kernel/signal.c b/kernel/signal.c index c6e4c83d..049a482 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2844,10 +2844,6 @@ enum siginfo_layout siginfo_layout(int sig, int si_code) if ((sig == SIGFPE) && (si_code == FPE_FIXME)) layout = SIL_FAULT; #endif -#ifdef BUS_FIXME - if ((sig == SIGBUS) && (si_code == BUS_FIXME)) - layout = SIL_FAULT; -#endif } return layout; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals @ 2018-03-08 16:37 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 16:37 UTC (permalink / raw) To: Dave Martin Cc: linux-arch, linux-api, James Morse, Eric W. Biederman, Catalin Marinas, linux-arm-kernel Hi Dave, On Thu, Mar 01, 2018 at 05:44:08PM +0000, Dave Martin wrote: > Currently, as reported by Eric, an invalid si_code value 0 is > passed in many signals delivered to userspace in response to faults > and other kernel errors. Typically 0 is passed when the fault is > insufficiently diagnosable or when there does not appear to be any > sensible alternative value to choose. This looks good to me. Please could you rebase it on top of for-next/core, so that I can apply it for 4.17? The other two patches in the series need an Ack for the core changes, so it would be worth dealing with those separately imo. Cheers, Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals @ 2018-03-08 16:37 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 16:37 UTC (permalink / raw) To: linux-arm-kernel Hi Dave, On Thu, Mar 01, 2018 at 05:44:08PM +0000, Dave Martin wrote: > Currently, as reported by Eric, an invalid si_code value 0 is > passed in many signals delivered to userspace in response to faults > and other kernel errors. Typically 0 is passed when the fault is > insufficiently diagnosable or when there does not appear to be any > sensible alternative value to choose. This looks good to me. Please could you rebase it on top of for-next/core, so that I can apply it for 4.17? The other two patches in the series need an Ack for the core changes, so it would be worth dealing with those separately imo. Cheers, Will ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals @ 2018-03-08 16:37 ` Will Deacon 0 siblings, 0 replies; 41+ messages in thread From: Will Deacon @ 2018-03-08 16:37 UTC (permalink / raw) To: Dave Martin Cc: linux-arm-kernel, linux-arch, linux-api, Eric W. Biederman, Catalin Marinas, James Morse Hi Dave, On Thu, Mar 01, 2018 at 05:44:08PM +0000, Dave Martin wrote: > Currently, as reported by Eric, an invalid si_code value 0 is > passed in many signals delivered to userspace in response to faults > and other kernel errors. Typically 0 is passed when the fault is > insufficiently diagnosable or when there does not appear to be any > sensible alternative value to choose. This looks good to me. Please could you rebase it on top of for-next/core, so that I can apply it for 4.17? The other two patches in the series need an Ack for the core changes, so it would be worth dealing with those separately imo. Cheers, Will ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2018-03-20 10:04 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-01 17:44 [PATCH v2 0/3] arm64: Fix invalid si_codes for fault signals Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-01 17:44 ` [PATCH v2 1/3] signal: Add FPE_FLTUNK si_code for undiagnosable fp exceptions Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-08 17:11 ` Will Deacon 2018-03-08 17:11 ` Will Deacon 2018-03-08 22:35 ` Eric W. Biederman 2018-03-08 22:35 ` Eric W. Biederman 2018-03-08 22:35 ` Eric W. Biederman 2018-03-01 17:44 ` [PATCH v2 2/3] arm64: fpsimd: Fix bad si_code for undiagnosed SIGFPE Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-08 17:11 ` Will Deacon 2018-03-08 17:11 ` Will Deacon 2018-03-08 17:11 ` Will Deacon 2018-03-08 22:40 ` Eric W. Biederman 2018-03-08 22:40 ` Eric W. Biederman 2018-03-08 22:40 ` Eric W. Biederman 2018-03-09 13:10 ` Will Deacon 2018-03-09 13:10 ` Will Deacon 2018-03-09 13:10 ` Will Deacon 2018-03-09 14:25 ` Dave Martin 2018-03-09 14:25 ` Dave Martin 2018-03-09 14:25 ` Dave Martin 2018-03-15 21:13 ` Eric W. Biederman 2018-03-15 21:13 ` Eric W. Biederman 2018-03-15 21:13 ` Eric W. Biederman 2018-03-20 10:04 ` Will Deacon 2018-03-20 10:04 ` Will Deacon 2018-03-20 10:04 ` Will Deacon 2018-03-08 22:37 ` Eric W. Biederman 2018-03-08 22:37 ` Eric W. Biederman 2018-03-08 22:37 ` Eric W. Biederman 2018-03-01 17:44 ` [PATCH v2 3/3] arm64: signal: Ensure si_code is valid for all fault signals Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-01 17:44 ` Dave Martin 2018-03-08 16:37 ` Will Deacon 2018-03-08 16:37 ` Will Deacon 2018-03-08 16:37 ` 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.