| Message ID | 1516623798-25001-2-git-send-email-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index e447283..77edb00 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -193,7 +193,8 @@ typedef struct siginfo { > #define FPE_FLTRES 6 /* floating point inexact result */ > #define FPE_FLTINV 7 /* floating point invalid operation */ > #define FPE_FLTSUB 8 /* subscript out of range */ > -#define NSIGFPE 8 > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ > +#define NSIGFPE 9 Minor nit here. At least before this is final I would really appreciate if you could rebase this on top of my unificiation of siginfo.h that I posted on linux-arch and is in my siginfo-next branch. As that already pushes NSIGFPE up to 13. Which would make this patch change NSIGFPE to 14 and allocate the number 14 for FPE_UNKNOWN Eric
On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: > > > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > > index e447283..77edb00 100644 > > --- a/include/uapi/asm-generic/siginfo.h > > +++ b/include/uapi/asm-generic/siginfo.h > > @@ -193,7 +193,8 @@ typedef struct siginfo { > > #define FPE_FLTRES 6 /* floating point inexact result */ > > #define FPE_FLTINV 7 /* floating point invalid operation */ > > #define FPE_FLTSUB 8 /* subscript out of range */ > > -#define NSIGFPE 8 > > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ > > +#define NSIGFPE 9 > > Minor nit here. > > At least before this is final I would really appreciate if you could > rebase this on top of my unificiation of siginfo.h that I posted on > linux-arch and is in my siginfo-next branch. > > As that already pushes NSIGFPE up to 13. > > Which would make this patch change NSIGFPE to 14 and allocate the number > 14 for FPE_UNKNOWN My bad -- I hadn't looked in detail at the whole series. However, the purpose of this as an RFC was to get feedback on whether adding FPE_UNKNOWN is considered acceptable at all from an API perspective -- the precise number doesn't matter for that discussion. Do you have any view on this? Cheers ---Dave
Dave Martin <Dave.Martin@arm.com> writes: > On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: >> >> > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h >> > index e447283..77edb00 100644 >> > --- a/include/uapi/asm-generic/siginfo.h >> > +++ b/include/uapi/asm-generic/siginfo.h >> > @@ -193,7 +193,8 @@ typedef struct siginfo { >> > #define FPE_FLTRES 6 /* floating point inexact result */ >> > #define FPE_FLTINV 7 /* floating point invalid operation */ >> > #define FPE_FLTSUB 8 /* subscript out of range */ >> > -#define NSIGFPE 8 >> > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ >> > +#define NSIGFPE 9 >> >> Minor nit here. >> >> At least before this is final I would really appreciate if you could >> rebase this on top of my unificiation of siginfo.h that I posted on >> linux-arch and is in my siginfo-next branch. >> >> As that already pushes NSIGFPE up to 13. >> >> Which would make this patch change NSIGFPE to 14 and allocate the number >> 14 for FPE_UNKNOWN > > My bad -- I hadn't looked in detail at the whole series. > > However, the purpose of this as an RFC was to get feedback on whether > adding FPE_UNKNOWN is considered acceptable at all from an API > perspective -- the precise number doesn't matter for that discussion. > > Do you have any view on this? That seems as good a solution as any too me. It is reality and it happens in the code and there are several places of the same form I would use it, just to get rid of the FPE_FIXME. Eric
From: ebiederm@xmission.com (Eric W. Biederman) Date: Tue, 23 Jan 2018 12:27:16 -0600 > Dave Martin <Dave.Martin@arm.com> writes: > >> On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: >> However, the purpose of this as an RFC was to get feedback on whether >> adding FPE_UNKNOWN is considered acceptable at all from an API >> perspective -- the precise number doesn't matter for that discussion. >> >> Do you have any view on this? > > That seems as good a solution as any too me. It is reality and it > happens in the code and there are several places of the same form I > would use it, just to get rid of the FPE_FIXME. Eric, feel free to do something similar on Sparc.
David Miller <davem@davemloft.net> writes: > From: ebiederm@xmission.com (Eric W. Biederman) > Date: Tue, 23 Jan 2018 12:27:16 -0600 > >> Dave Martin <Dave.Martin@arm.com> writes: >> >>> On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: >>> However, the purpose of this as an RFC was to get feedback on whether >>> adding FPE_UNKNOWN is considered acceptable at all from an API >>> perspective -- the precise number doesn't matter for that discussion. >>> >>> Do you have any view on this? >> >> That seems as good a solution as any too me. It is reality and it >> happens in the code and there are several places of the same form I >> would use it, just to get rid of the FPE_FIXME. > > Eric, feel free to do something similar on Sparc. Will do. This sounds like a good solution for this weird corner case, that appears on multiple architectures. Eric
On Tue, Jan 23, 2018 at 01:44:19PM -0600, Eric W. Biederman wrote: > David Miller <davem@davemloft.net> writes: > > > From: ebiederm@xmission.com (Eric W. Biederman) > > Date: Tue, 23 Jan 2018 12:27:16 -0600 > > > >> Dave Martin <Dave.Martin@arm.com> writes: > >> > >>> On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: > >>> However, the purpose of this as an RFC was to get feedback on whether > >>> adding FPE_UNKNOWN is considered acceptable at all from an API > >>> perspective -- the precise number doesn't matter for that discussion. > >>> > >>> Do you have any view on this? > >> > >> That seems as good a solution as any too me. It is reality and it > >> happens in the code and there are several places of the same form I > >> would use it, just to get rid of the FPE_FIXME. > > > > Eric, feel free to do something similar on Sparc. > > Will do. > > This sounds like a good solution for this weird corner case, that > appears on multiple architectures. OK, I'll rebase my patches onto your tree (though trivial here) and repost. I'm still waiting for feeback on the Arm specifics, but FPE_UNKNOWN could be picked up independently of that. Cheers ---Dave
On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: > > > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > > index e447283..77edb00 100644 > > --- a/include/uapi/asm-generic/siginfo.h > > +++ b/include/uapi/asm-generic/siginfo.h > > @@ -193,7 +193,8 @@ typedef struct siginfo { > > #define FPE_FLTRES 6 /* floating point inexact result */ > > #define FPE_FLTINV 7 /* floating point invalid operation */ > > #define FPE_FLTSUB 8 /* subscript out of range */ > > -#define NSIGFPE 8 > > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ > > +#define NSIGFPE 9 > > Minor nit here. > > At least before this is final I would really appreciate if you could > rebase this on top of my unificiation of siginfo.h that I posted on > linux-arch and is in my siginfo-next branch. > > As that already pushes NSIGFPE up to 13. > > Which would make this patch change NSIGFPE to 14 and allocate the number > 14 for FPE_UNKNOWN Looking at this, I note a few things: * For consistent naming, FPE_FLTUNK might be a better name for FPE_UNKNOWN. FPE_FLTUNK seems generic, tempting me to insert it as number 9 (since only the numbers up to 8 are ABI just now). (The temptation to call it FPE_FLUNK is strong, but I can't argue that's consistent...) * No distinction is drawn between generic and arch-dependent codes here, so NSIGFPE will typically be too big. The generic siginfo handling code can detect random garbage in si_code this way, but off-by-ones or misused arch-specific codes may slip through. In particular, new x86-specific FPE_* codes will likely be invisible to the BUILD_BUG_ON()s in arch/x86/kernel/signal_compat.c unless so many are added that x86 overtakes ia64. * Should we reserve space for future generic codes (say up to 15)? Downside: si_code validation is not a simple matter of checking <= NSIGFPE in that case. (Though <= is still better than no check at all, and no worse than the current situation.) * What are NSIGFPE etc. doing in this header? These aren't specified by POSIX and I'm not sure what userspace would legitimately use them for... though it may be too late to change this now. Most instances on codeseaarch.debian.net are the kernel, copies of kernel headers, and translated versions of kernel headers. It's hard to be exhaustive though. We could have something like this: #define FPE_FLTUNK 9 #define __NSIGFPE_GENERIC 9 #define NSIGFPE __NSIGFPE_GENERIC /* si_code <= 15 reserved for arch-independent codes */ #if defined(__frv__) # define FPE_MDAOF 16 # undef NSIGFPE # define NSIGFPE 16 #elif define(__ia64__) # define __FPE_DECOVF 16 # define __FPE_DECDIV 17 # define __FPE_DECERR 18 # define __FPR_INVASC 19 # undef NSIGFPE # define NSIGFPE 19 #endif (Avoiding a (base + offset) approach for the arch codes, since that would make it look like the codes can be renumbered safely without breaking anything). The generic vs. arch vs. NSIGFOO problem already exists for other signals. We could take a similar approach for those, but OTOH it may just not be worth the effort. Cheers ---Dave
Dave Martin <Dave.Martin@arm.com> writes: > On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: >> >> > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h >> > index e447283..77edb00 100644 >> > --- a/include/uapi/asm-generic/siginfo.h >> > +++ b/include/uapi/asm-generic/siginfo.h >> > @@ -193,7 +193,8 @@ typedef struct siginfo { >> > #define FPE_FLTRES 6 /* floating point inexact result */ >> > #define FPE_FLTINV 7 /* floating point invalid operation */ >> > #define FPE_FLTSUB 8 /* subscript out of range */ >> > -#define NSIGFPE 8 >> > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ >> > +#define NSIGFPE 9 >> >> Minor nit here. >> >> At least before this is final I would really appreciate if you could >> rebase this on top of my unificiation of siginfo.h that I posted on >> linux-arch and is in my siginfo-next branch. >> >> As that already pushes NSIGFPE up to 13. >> >> Which would make this patch change NSIGFPE to 14 and allocate the number >> 14 for FPE_UNKNOWN > > Looking at this, I note a few things: > > * For consistent naming, FPE_FLTUNK might be a better name for > FPE_UNKNOWN. > > FPE_FLTUNK seems generic, tempting me to insert it as number 9 > (since only the numbers up to 8 are ABI just now). Except on ia64 and frv. And who knows we might need it on one of those architectures as well. > (The temptation to call it FPE_FLUNK is strong, but I can't argue > that's consistent...) I totally understand the temptation. > * No distinction is drawn between generic and arch-dependent codes > here, so NSIGFPE will typically be too big. The generic siginfo > handling code can detect random garbage in si_code this way, but > off-by-ones or misused arch-specific codes may slip through. > > In particular, new x86-specific FPE_* codes will likely be > invisible to the BUILD_BUG_ON()s in arch/x86/kernel/signal_compat.c > unless so many are added that x86 overtakes ia64. Long ago in a far off time, we had arch dependent system call numbers and the like because that provided ABI compatibility with the existing unix on the platform. I don't see any of that with the siginfo si_codes. In most cases they are arch dependent extensions which is silly. We should have unconditionally extended the si_codes for all architectures in case another architecture needs that si_code. The fact we now have battling meanings for si_codes depending on the architecture is an unfortunate mess. So to me it looks most maintainable going forward to declare that all si_codes should be allocated generically, from the same number space, in the same header file. While we live with the existing historic mess. > * Should we reserve space for future generic codes (say up to 15)? > Downside: si_code validation is not a simple matter of checking > <= NSIGFPE in that case. (Though <= is still better than no > check at all, and no worse than the current situation.) I think new si_codes should be allocated where there are not conflicts on any architecture. Just in case they are useful on another architecture in the future. > * What are NSIGFPE etc. doing in this header? These aren't specified > by POSIX and I'm not sure what userspace would legitimately use them > for... though it may be too late to change this now. > > Most instances on codeseaarch.debian.net are the kernel, copies > of kernel headers, and translated versions of kernel headers. > It's hard to be exhaustive though. > > > We could have something like this: > > #define FPE_FLTUNK 9 > #define __NSIGFPE_GENERIC 9 > #define NSIGFPE __NSIGFPE_GENERIC > > /* si_code <= 15 reserved for arch-independent codes */ > > #if defined(__frv__) > > # define FPE_MDAOF 16 > # undef NSIGFPE > # define NSIGFPE 16 > > #elif define(__ia64__) > > # define __FPE_DECOVF 16 > # define __FPE_DECDIV 17 > # define __FPE_DECERR 18 > # define __FPR_INVASC 19 > # undef NSIGFPE > # define NSIGFPE 19 > > #endif > > (Avoiding a (base + offset) approach for the arch codes, since that > would make it look like the codes can be renumbered safely without > breaking anything). > > The generic vs. arch vs. NSIGFOO problem already exists for other > signals. We could take a similar approach for those, but OTOH it > may just not be worth the effort. What I have tried to do in my merger is discurage the idea that there are any arch specific si_codes. To set NSIGXXX to the largest value from any of the architectures. And to encourage new si_codes get allocated after the current NSIGXXX. So that they will work on all architectures. It is all a bit of a mess, but one unified mess seems like the best we can do right now. Eric
On Wed, Jan 24, 2018 at 10:47:36AM -0600, Eric W. Biederman wrote: > Dave Martin <Dave.Martin@arm.com> writes: > > > On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: > >> > >> > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > >> > index e447283..77edb00 100644 > >> > --- a/include/uapi/asm-generic/siginfo.h > >> > +++ b/include/uapi/asm-generic/siginfo.h > >> > @@ -193,7 +193,8 @@ typedef struct siginfo { > >> > #define FPE_FLTRES 6 /* floating point inexact result */ > >> > #define FPE_FLTINV 7 /* floating point invalid operation */ > >> > #define FPE_FLTSUB 8 /* subscript out of range */ > >> > -#define NSIGFPE 8 > >> > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ > >> > +#define NSIGFPE 9 > >> > >> Minor nit here. > >> > >> At least before this is final I would really appreciate if you could > >> rebase this on top of my unificiation of siginfo.h that I posted on > >> linux-arch and is in my siginfo-next branch. > >> > >> As that already pushes NSIGFPE up to 13. > >> > >> Which would make this patch change NSIGFPE to 14 and allocate the number > >> 14 for FPE_UNKNOWN > > > > Looking at this, I note a few things: > > > > * For consistent naming, FPE_FLTUNK might be a better name for > > FPE_UNKNOWN. > > > > FPE_FLTUNK seems generic, tempting me to insert it as number 9 > > (since only the numbers up to 8 are ABI just now). > > Except on ia64 and frv. And who knows we might need it on one of those > architectures as well. I thought those weren't actually upstreamed yet... > > (The temptation to call it FPE_FLUNK is strong, but I can't argue > > that's consistent...) > > I totally understand the temptation. > > > * No distinction is drawn between generic and arch-dependent codes > > here, so NSIGFPE will typically be too big. The generic siginfo > > handling code can detect random garbage in si_code this way, but > > off-by-ones or misused arch-specific codes may slip through. > > > > In particular, new x86-specific FPE_* codes will likely be > > invisible to the BUILD_BUG_ON()s in arch/x86/kernel/signal_compat.c > > unless so many are added that x86 overtakes ia64. > > Long ago in a far off time, we had arch dependent system call numbers > and the like because that provided ABI compatibility with the existing > unix on the platform. > > I don't see any of that with the siginfo si_codes. In most cases > they are arch dependent extensions which is silly. We should have > unconditionally extended the si_codes for all architectures in case > another architecture needs that si_code. > > The fact we now have battling meanings for si_codes depending on the > architecture is an unfortunate mess. > > So to me it looks most maintainable going forward to declare that all > si_codes should be allocated generically, from the same number space, > in the same header file. While we live with the existing historic > mess. I guess that's fair enough. This also provides a consistent interpretation for NSIGXXX. > > * Should we reserve space for future generic codes (say up to 15)? > > Downside: si_code validation is not a simple matter of checking > > <= NSIGFPE in that case. (Though <= is still better than no > > check at all, and no worse than the current situation.) > > I think new si_codes should be allocated where there are not conflicts > on any architecture. Just in case they are useful on another > architecture in the future. > > > * What are NSIGFPE etc. doing in this header? These aren't specified > > by POSIX and I'm not sure what userspace would legitimately use them > > for... though it may be too late to change this now. > > > > Most instances on codeseaarch.debian.net are the kernel, copies > > of kernel headers, and translated versions of kernel headers. > > It's hard to be exhaustive though. > > > > > > We could have something like this: > > > > #define FPE_FLTUNK 9 > > #define __NSIGFPE_GENERIC 9 > > #define NSIGFPE __NSIGFPE_GENERIC > > > > /* si_code <= 15 reserved for arch-independent codes */ > > > > #if defined(__frv__) > > > > # define FPE_MDAOF 16 > > # undef NSIGFPE > > # define NSIGFPE 16 > > > > #elif define(__ia64__) > > > > # define __FPE_DECOVF 16 > > # define __FPE_DECDIV 17 > > # define __FPE_DECERR 18 > > # define __FPR_INVASC 19 > > # undef NSIGFPE > > # define NSIGFPE 19 > > > > #endif > > > > (Avoiding a (base + offset) approach for the arch codes, since that > > would make it look like the codes can be renumbered safely without > > breaking anything). > > > > The generic vs. arch vs. NSIGFOO problem already exists for other > > signals. We could take a similar approach for those, but OTOH it > > may just not be worth the effort. > > What I have tried to do in my merger is discurage the idea that there > are any arch specific si_codes. To set NSIGXXX to the largest value > from any of the architectures. And to encourage new si_codes get > allocated after the current NSIGXXX. So that they will work on all > architectures. > > It is all a bit of a mess, but one unified mess seems like the best we > can do right now. That sounds fair, now that I have a better understanding of the context for all this. If the policy is that all the codes are generic (even if not all can happen on all arches) then FPE_FLTUNK may as well be 14. Cheers ---Dave
Dave Martin <Dave.Martin@arm.com> writes: > On Wed, Jan 24, 2018 at 10:47:36AM -0600, Eric W. Biederman wrote: >> Dave Martin <Dave.Martin@arm.com> writes: >> >> > On Mon, Jan 22, 2018 at 03:13:08PM -0600, Eric W. Biederman wrote: >> >> >> >> > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h >> >> > index e447283..77edb00 100644 >> >> > --- a/include/uapi/asm-generic/siginfo.h >> >> > +++ b/include/uapi/asm-generic/siginfo.h >> >> > @@ -193,7 +193,8 @@ typedef struct siginfo { >> >> > #define FPE_FLTRES 6 /* floating point inexact result */ >> >> > #define FPE_FLTINV 7 /* floating point invalid operation */ >> >> > #define FPE_FLTSUB 8 /* subscript out of range */ >> >> > -#define NSIGFPE 8 >> >> > +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ >> >> > +#define NSIGFPE 9 >> >> >> >> Minor nit here. >> >> >> >> At least before this is final I would really appreciate if you could >> >> rebase this on top of my unificiation of siginfo.h that I posted on >> >> linux-arch and is in my siginfo-next branch. >> >> >> >> As that already pushes NSIGFPE up to 13. >> >> >> >> Which would make this patch change NSIGFPE to 14 and allocate the number >> >> 14 for FPE_UNKNOWN >> > >> > Looking at this, I note a few things: >> > >> > * For consistent naming, FPE_FLTUNK might be a better name for >> > FPE_UNKNOWN. >> > >> > FPE_FLTUNK seems generic, tempting me to insert it as number 9 >> > (since only the numbers up to 8 are ABI just now). >> >> Except on ia64 and frv. And who knows we might need it on one of those >> architectures as well. > > I thought those weren't actually upstreamed yet... Oh no. Those have been upstream for a decade or so. They just have not been in one unified file. >> > (The temptation to call it FPE_FLUNK is strong, but I can't argue >> > that's consistent...) >> >> I totally understand the temptation. >> >> > * No distinction is drawn between generic and arch-dependent codes >> > here, so NSIGFPE will typically be too big. The generic siginfo >> > handling code can detect random garbage in si_code this way, but >> > off-by-ones or misused arch-specific codes may slip through. >> > >> > In particular, new x86-specific FPE_* codes will likely be >> > invisible to the BUILD_BUG_ON()s in arch/x86/kernel/signal_compat.c >> > unless so many are added that x86 overtakes ia64. >> >> Long ago in a far off time, we had arch dependent system call numbers >> and the like because that provided ABI compatibility with the existing >> unix on the platform. >> >> I don't see any of that with the siginfo si_codes. In most cases >> they are arch dependent extensions which is silly. We should have >> unconditionally extended the si_codes for all architectures in case >> another architecture needs that si_code. >> >> The fact we now have battling meanings for si_codes depending on the >> architecture is an unfortunate mess. >> >> So to me it looks most maintainable going forward to declare that all >> si_codes should be allocated generically, from the same number space, >> in the same header file. While we live with the existing historic >> mess. > > I guess that's fair enough. This also provides a consistent > interpretation for NSIGXXX. > >> > * Should we reserve space for future generic codes (say up to 15)? >> > Downside: si_code validation is not a simple matter of checking >> > <= NSIGFPE in that case. (Though <= is still better than no >> > check at all, and no worse than the current situation.) >> >> I think new si_codes should be allocated where there are not conflicts >> on any architecture. Just in case they are useful on another >> architecture in the future. >> >> > * What are NSIGFPE etc. doing in this header? These aren't specified >> > by POSIX and I'm not sure what userspace would legitimately use them >> > for... though it may be too late to change this now. >> > >> > Most instances on codeseaarch.debian.net are the kernel, copies >> > of kernel headers, and translated versions of kernel headers. >> > It's hard to be exhaustive though. >> > >> > >> > We could have something like this: >> > >> > #define FPE_FLTUNK 9 >> > #define __NSIGFPE_GENERIC 9 >> > #define NSIGFPE __NSIGFPE_GENERIC >> > >> > /* si_code <= 15 reserved for arch-independent codes */ >> > >> > #if defined(__frv__) >> > >> > # define FPE_MDAOF 16 >> > # undef NSIGFPE >> > # define NSIGFPE 16 >> > >> > #elif define(__ia64__) >> > >> > # define __FPE_DECOVF 16 >> > # define __FPE_DECDIV 17 >> > # define __FPE_DECERR 18 >> > # define __FPR_INVASC 19 >> > # undef NSIGFPE >> > # define NSIGFPE 19 >> > >> > #endif >> > >> > (Avoiding a (base + offset) approach for the arch codes, since that >> > would make it look like the codes can be renumbered safely without >> > breaking anything). >> > >> > The generic vs. arch vs. NSIGFOO problem already exists for other >> > signals. We could take a similar approach for those, but OTOH it >> > may just not be worth the effort. >> >> What I have tried to do in my merger is discurage the idea that there >> are any arch specific si_codes. To set NSIGXXX to the largest value >> from any of the architectures. And to encourage new si_codes get >> allocated after the current NSIGXXX. So that they will work on all >> architectures. >> >> It is all a bit of a mess, but one unified mess seems like the best we >> can do right now. > > That sounds fair, now that I have a better understanding of the context > for all this. > > If the policy is that all the codes are generic (even if not all can > happen on all arches) then FPE_FLTUNK may as well be 14. Exactly. Eric
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 014d7d8..c585e91 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -220,6 +220,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/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index fae81f7..2d6ba9e 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 = 0; - - 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_UNKNOWN; + + 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; diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index e447283..77edb00 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -193,7 +193,8 @@ typedef struct siginfo { #define FPE_FLTRES 6 /* floating point inexact result */ #define FPE_FLTINV 7 /* floating point invalid operation */ #define FPE_FLTSUB 8 /* subscript out of range */ -#define NSIGFPE 8 +#define FPE_UNKNOWN 9 /* undiagnosed floating-point exception */ +#define NSIGFPE 9 /* * SIGSEGV si_codes
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/kernel/fpsimd.c | 27 +++++++++++++++------------ include/uapi/asm-generic/siginfo.h | 3 ++- 3 files changed, 26 insertions(+), 13 deletions(-)