| Message ID | d93b503a926f45e752178bb61f451a426a558260.1597720138.git.pcc@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: expose FAR_EL1 tag bits in siginfo | expand |
On Mon, Aug 17, 2020 at 08:33:49PM -0700, Peter Collingbourne wrote: > This bit will never be supported in the uapi. The purpose of this flag > bit is to allow userspace to distinguish an old kernel that does not > clear unknown sa_flags bits from a kernel that supports every flag bit. > > In other words, if userspace finds that this bit remains set in > oldact.sa_flags, it means that the kernel cannot be trusted to have > cleared unknown flag bits from sa_flags, so no assumptions about flag > bit support can be made. > > Signed-off-by: Peter Collingbourne <pcc@google.com> > --- > View this change in Gerrit: https://linux-review.googlesource.com/q/Ic2501ad150a3a79c1cf27fb8c99be342e9dffbcb > > include/uapi/asm-generic/signal-defs.h | 7 +++++++ > kernel/signal.c | 6 ++++++ > 2 files changed, 13 insertions(+) > > diff --git a/include/uapi/asm-generic/signal-defs.h b/include/uapi/asm-generic/signal-defs.h > index 91000b6b97e0..c30a9c1a77b2 100644 > --- a/include/uapi/asm-generic/signal-defs.h > +++ b/include/uapi/asm-generic/signal-defs.h > @@ -13,6 +13,12 @@ > * SA_RESETHAND clears the handler when the signal is delivered. > * SA_NOCLDWAIT flag on SIGCHLD to inhibit zombies. > * SA_NODEFER prevents the current signal from being masked in the handler. > + * SA_UNSUPPORTED is a flag bit that will never be supported. Kernels from > + * before the introduction of SA_UNSUPPORTED did not clear unknown bits from > + * sa_flags when read using the oldact argument to sigaction and rt_sigaction, > + * so this bit allows flag bit support to be detected from userspace while > + * allowing an old kernel to be distinguished from a kernel that supports every > + * flag bit. > * > * SA_ONESHOT and SA_NOMASK are the historical Linux names for the Single > * Unix names RESETHAND and NODEFER respectively. > @@ -42,6 +48,7 @@ > * The following bits are used in architecture-specific SA_* definitions and > * should be avoided for new generic flags: 3, 4, 5, 6, 7, 8, 9, 16, 24, 25, 26. > */ > +#define SA_UNSUPPORTED 0x00000400 This concept confused me a bit initially, since in a sense this flag is supported, just with a rather peculiar meaning. Since the main (only) purpose of this bit will be to check whether SA_XFLAGS is actually supported, I wonder whether it makes sense to weld the two together, say: #define SA_REQUEST_XFLAGS 0x00000c00 #define SA_XFLAGS_MASK 0x00000c00 #define SA_HAVE_XFLAGS 0x00000800 This is a departure from the current style of definitions though. sa.sa_flags |= SA_REQUEST_XFLAGS; sigaction(..., &sa, &sa); if ((sa.sa_flags & SA_XFLAGS_MASK) == SA_HAVE_XFLAGS) /* xflags available */ This would require some juggling of the way SA_UAPI_FLAGS works though. Maybe not worth it, so long as the semantics get clearly documented. [...] Cheers ---Dave
On Wed, Aug 19, 2020 at 7:51 AM Dave Martin <Dave.Martin@arm.com> wrote: > > On Mon, Aug 17, 2020 at 08:33:49PM -0700, Peter Collingbourne wrote: > > This bit will never be supported in the uapi. The purpose of this flag > > bit is to allow userspace to distinguish an old kernel that does not > > clear unknown sa_flags bits from a kernel that supports every flag bit. > > > > In other words, if userspace finds that this bit remains set in > > oldact.sa_flags, it means that the kernel cannot be trusted to have > > cleared unknown flag bits from sa_flags, so no assumptions about flag > > bit support can be made. > > > > Signed-off-by: Peter Collingbourne <pcc@google.com> > > --- > > View this change in Gerrit: https://linux-review.googlesource.com/q/Ic2501ad150a3a79c1cf27fb8c99be342e9dffbcb > > > > include/uapi/asm-generic/signal-defs.h | 7 +++++++ > > kernel/signal.c | 6 ++++++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/include/uapi/asm-generic/signal-defs.h b/include/uapi/asm-generic/signal-defs.h > > index 91000b6b97e0..c30a9c1a77b2 100644 > > --- a/include/uapi/asm-generic/signal-defs.h > > +++ b/include/uapi/asm-generic/signal-defs.h > > @@ -13,6 +13,12 @@ > > * SA_RESETHAND clears the handler when the signal is delivered. > > * SA_NOCLDWAIT flag on SIGCHLD to inhibit zombies. > > * SA_NODEFER prevents the current signal from being masked in the handler. > > + * SA_UNSUPPORTED is a flag bit that will never be supported. Kernels from > > + * before the introduction of SA_UNSUPPORTED did not clear unknown bits from > > + * sa_flags when read using the oldact argument to sigaction and rt_sigaction, > > + * so this bit allows flag bit support to be detected from userspace while > > + * allowing an old kernel to be distinguished from a kernel that supports every > > + * flag bit. > > * > > * SA_ONESHOT and SA_NOMASK are the historical Linux names for the Single > > * Unix names RESETHAND and NODEFER respectively. > > @@ -42,6 +48,7 @@ > > * The following bits are used in architecture-specific SA_* definitions and > > * should be avoided for new generic flags: 3, 4, 5, 6, 7, 8, 9, 16, 24, 25, 26. > > */ > > +#define SA_UNSUPPORTED 0x00000400 > > This concept confused me a bit initially, since in a sense this flag is > supported, just with a rather peculiar meaning. Hmm. Maybe it should be named "SA_UNKNOWN" to mean that the bit will always be "unknown" to the kernel in the sense that it shall be treated in the same way as any other "unknown" bit. Then we can define the kernel's behavior in terms of what happens if a bit is "known". I don't know if this is just shuffling terms around though. At any rate, this seems like a problem to be solved with documentation. > Since the main (only) purpose of this bit will be to check whether I wouldn't necessarily say that it is the only purpose. If another new sa_flags bit were to be introduced in the future, SA_UN(whatever) could be used to detect kernel support for that bit in the same way as SA_XFLAGS. > SA_XFLAGS is actually supported, I wonder whether it makes sense to weld > the two together, say: > > #define SA_REQUEST_XFLAGS 0x00000c00 > #define SA_XFLAGS_MASK 0x00000c00 > #define SA_HAVE_XFLAGS 0x00000800 > > This is a departure from the current style of definitions though. > > sa.sa_flags |= SA_REQUEST_XFLAGS; > sigaction(..., &sa, &sa); > if ((sa.sa_flags & SA_XFLAGS_MASK) == SA_HAVE_XFLAGS) > /* xflags available */ > > > This would require some juggling of the way SA_UAPI_FLAGS works though. > Maybe not worth it, so long as the semantics get clearly documented. I'm not sure about this. I personally think that it would be clearer to keep the flags orthogonal. Peter
On Wed, Aug 19, 2020 at 05:23:25PM -0700, Peter Collingbourne wrote: > On Wed, Aug 19, 2020 at 7:51 AM Dave Martin <Dave.Martin@arm.com> wrote: > > > > On Mon, Aug 17, 2020 at 08:33:49PM -0700, Peter Collingbourne wrote: > > > This bit will never be supported in the uapi. The purpose of this flag > > > bit is to allow userspace to distinguish an old kernel that does not > > > clear unknown sa_flags bits from a kernel that supports every flag bit. > > > > > > In other words, if userspace finds that this bit remains set in > > > oldact.sa_flags, it means that the kernel cannot be trusted to have > > > cleared unknown flag bits from sa_flags, so no assumptions about flag > > > bit support can be made. > > > > > > Signed-off-by: Peter Collingbourne <pcc@google.com> > > > --- > > > View this change in Gerrit: https://linux-review.googlesource.com/q/Ic2501ad150a3a79c1cf27fb8c99be342e9dffbcb > > > > > > include/uapi/asm-generic/signal-defs.h | 7 +++++++ > > > kernel/signal.c | 6 ++++++ > > > 2 files changed, 13 insertions(+) > > > > > > diff --git a/include/uapi/asm-generic/signal-defs.h b/include/uapi/asm-generic/signal-defs.h > > > index 91000b6b97e0..c30a9c1a77b2 100644 > > > --- a/include/uapi/asm-generic/signal-defs.h > > > +++ b/include/uapi/asm-generic/signal-defs.h > > > @@ -13,6 +13,12 @@ > > > * SA_RESETHAND clears the handler when the signal is delivered. > > > * SA_NOCLDWAIT flag on SIGCHLD to inhibit zombies. > > > * SA_NODEFER prevents the current signal from being masked in the handler. > > > + * SA_UNSUPPORTED is a flag bit that will never be supported. Kernels from > > > + * before the introduction of SA_UNSUPPORTED did not clear unknown bits from > > > + * sa_flags when read using the oldact argument to sigaction and rt_sigaction, > > > + * so this bit allows flag bit support to be detected from userspace while > > > + * allowing an old kernel to be distinguished from a kernel that supports every > > > + * flag bit. > > > * > > > * SA_ONESHOT and SA_NOMASK are the historical Linux names for the Single > > > * Unix names RESETHAND and NODEFER respectively. > > > @@ -42,6 +48,7 @@ > > > * The following bits are used in architecture-specific SA_* definitions and > > > * should be avoided for new generic flags: 3, 4, 5, 6, 7, 8, 9, 16, 24, 25, 26. > > > */ > > > +#define SA_UNSUPPORTED 0x00000400 > > > > This concept confused me a bit initially, since in a sense this flag is > > supported, just with a rather peculiar meaning. > > Hmm. Maybe it should be named "SA_UNKNOWN" to mean that the bit will > always be "unknown" to the kernel in the sense that it shall be > treated in the same way as any other "unknown" bit. Then we can define > the kernel's behavior in terms of what happens if a bit is "known". I > don't know if this is just shuffling terms around though. At any rate, > this seems like a problem to be solved with documentation. > > > Since the main (only) purpose of this bit will be to check whether > > I wouldn't necessarily say that it is the only purpose. If another new > sa_flags bit were to be introduced in the future, SA_UN(whatever) > could be used to detect kernel support for that bit in the same way as > SA_XFLAGS. > > > SA_XFLAGS is actually supported, I wonder whether it makes sense to weld > > the two together, say: > > > > #define SA_REQUEST_XFLAGS 0x00000c00 > > #define SA_XFLAGS_MASK 0x00000c00 > > #define SA_HAVE_XFLAGS 0x00000800 > > > > This is a departure from the current style of definitions though. > > > > sa.sa_flags |= SA_REQUEST_XFLAGS; > > sigaction(..., &sa, &sa); > > if ((sa.sa_flags & SA_XFLAGS_MASK) == SA_HAVE_XFLAGS) > > /* xflags available */ > > > > > > This would require some juggling of the way SA_UAPI_FLAGS works though. > > Maybe not worth it, so long as the semantics get clearly documented. > > I'm not sure about this. I personally think that it would be clearer > to keep the flags orthogonal. Fair enough. I didn't think my approach was a whole lot better tbh. Cheers ---Dave
diff --git a/include/uapi/asm-generic/signal-defs.h b/include/uapi/asm-generic/signal-defs.h index 91000b6b97e0..c30a9c1a77b2 100644 --- a/include/uapi/asm-generic/signal-defs.h +++ b/include/uapi/asm-generic/signal-defs.h @@ -13,6 +13,12 @@ * SA_RESETHAND clears the handler when the signal is delivered. * SA_NOCLDWAIT flag on SIGCHLD to inhibit zombies. * SA_NODEFER prevents the current signal from being masked in the handler. + * SA_UNSUPPORTED is a flag bit that will never be supported. Kernels from + * before the introduction of SA_UNSUPPORTED did not clear unknown bits from + * sa_flags when read using the oldact argument to sigaction and rt_sigaction, + * so this bit allows flag bit support to be detected from userspace while + * allowing an old kernel to be distinguished from a kernel that supports every + * flag bit. * * SA_ONESHOT and SA_NOMASK are the historical Linux names for the Single * Unix names RESETHAND and NODEFER respectively. @@ -42,6 +48,7 @@ * The following bits are used in architecture-specific SA_* definitions and * should be avoided for new generic flags: 3, 4, 5, 6, 7, 8, 9, 16, 24, 25, 26. */ +#define SA_UNSUPPORTED 0x00000400 #define SA_NOMASK SA_NODEFER #define SA_ONESHOT SA_RESETHAND diff --git a/kernel/signal.c b/kernel/signal.c index 348b7981f1ff..664a6c31137e 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -3984,6 +3984,12 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact) if (oact) *oact = *k; + /* + * Make sure that we never accidentally claim to support SA_UNSUPPORTED, + * e.g. by having an architecture use the bit in their uapi. + */ + BUILD_BUG_ON(SA_UAPI_FLAGS & SA_UNSUPPORTED); + /* * Clear unknown flag bits in order to allow userspace to detect missing * support for flag bits and to allow the kernel to use non-uapi bits
This bit will never be supported in the uapi. The purpose of this flag bit is to allow userspace to distinguish an old kernel that does not clear unknown sa_flags bits from a kernel that supports every flag bit. In other words, if userspace finds that this bit remains set in oldact.sa_flags, it means that the kernel cannot be trusted to have cleared unknown flag bits from sa_flags, so no assumptions about flag bit support can be made. Signed-off-by: Peter Collingbourne <pcc@google.com> --- View this change in Gerrit: https://linux-review.googlesource.com/q/Ic2501ad150a3a79c1cf27fb8c99be342e9dffbcb include/uapi/asm-generic/signal-defs.h | 7 +++++++ kernel/signal.c | 6 ++++++ 2 files changed, 13 insertions(+)