On Fri, Nov 13, 2020 at 05:41:32PM -0800, Peter Collingbourne wrote: > --- > These features are implemented in this patch series: > https://lore.kernel.org/linux-arm-kernel/cover.1605235762.git.pcc@xxxxxxxxxx/ > which is still under review, so the patch should not be applied > yet. Good to see this -- I was vaguely intending to write something when the patch landed, so this saves me a job :) > man2/sigaction.2 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/man2/sigaction.2 b/man2/sigaction.2 > index 6a8142324..82fb69e26 100644 > --- a/man2/sigaction.2 > +++ b/man2/sigaction.2 > @@ -250,6 +250,19 @@ This flag is meaningful only when establishing a signal handler. > .\" .I sa_sigaction > .\" field was added in Linux 2.1.86.) > .\" > +.TP > +.BR SA_UNSUPPORTED " (since Linux 5.x)" > +This flag bit will never be supported by the kernel. It is used as > +part of the flag support detection protocol described below. It is supported, just with a special behaviour that only affects the sa_flags field of oact. It's also a bit wried to say that SA_UNSUPPORTED is supported "since some specific version of Linux." If we define the special behaviour for this flag as "being supported", then saying "since Linux 5.x" just works naturally. Can we make a concise statement here about what the flag does and how to use it? It's preferable not to bury specification details in wordy rationale sections unless there's really no way to describe the behaviour concisely. Maybe: --8<-- Check for extended sa_flags support. If the attempt to register a handler with this flag set in act->sa_flags succeeds, and an immediately subsequent sigaction() call specifying the same signal number n and with non-NULL oact yields SA_UNSUPPORTED .I clear in oact->sa_flags, then extended sa_flags are supported. If the handler registration attempt fails, or if the subsequent sigaction() call yields SA_UNSUPPORTED .I set in oact->sa_flags, then the caller must not assume that extended sa_flags are supported. -->8-- Also, would it be a good idea to add something like the following? --8<-- This flag may be specified alongside extended sa_flags whose meaning depends on the result of the check. Since the behaviour of the signal handler cannot be guaranteed unless the check passes, it is wise to block the affected signal while registering the handler and performing the check in this case. -->8-- Also, can we reference the "Detecting flag support in sa_flags" by name? It may be hard for people to find otherwise... > +.TP > +.BR SA_EXPOSE_TAGBITS " (since Linux 5.x)" Shouldn't we mention the probe requirement here? --8<-- Requires extended sa_flags support; otherwise, the effect of this flag is unpecified. See <section> [or SA_UNSUPPORTED if we think that's the more concise description to refer to] for details. -->8-- > +Normally, when delivering a signal, an architecture-specific > +set of tag bits are cleared from the Maybe "address tag bits", to make it clearer which bits we're talking about. (But you're about to mention si_addr anyway, so perhaps this clarification is redundant.) > +.I si_addr > +field of > +.IR siginfo_t . > +If this flag is set, the tag bits will be preserved in Should we say "any available tag bits" or "an architecture-specific subset of the tag bits" or similar? This would cover us for the case where some arch can't always provide them all (which I think is the case for MTE on arm64? ... but my memory is hazy). > +.IR si_addr . > .SS The siginfo_t argument to a SA_SIGINFO handler > When the > .B SA_SIGINFO > @@ -833,6 +846,55 @@ Triggered by a > .BR seccomp (2) > filter rule. > .RE > +.SS Detecting flag support in sa_flags (Could be renamed to "Extended sa_flags support" if we want to be more concise.) > +The Linux kernel supports a mechanism for programs to detect kernel > +support for > +.B SA_* > +flags in > +.IR sa_flags . > +This mechanism is quite subtle for backwards compatibility reasons > +related to the historical behavior of the kernel. Can we avoid the word "subtle"? To me, this suggests a euphemism for "doesn't work reliably", or "nobody knows whether it works". Being more explicit about the actual reason for this design might help, since the reason is actually fairly simple. Maybe: --8<-- Historically, the sigaction(2) call on Linux accepted unknown bits set in act->sa_flags without error, and a second sigaction() call would typically leave those bits set in oact->sa_flags. This means that support for new flags cannot be detected simply by setting a flag in sa_flags. -->8-- > + > +Starting with Linux 5.x, the kernel will clear any unrecognized bits > +from the > +.I sa_flags > +value returned via > +.I oldact > +if those bits were set when the signal handler was originally installed. > +Therefore, a program that only needs to be compatible with Linux 5.x > +and above may test for flag bit support by issuing a second call to > +.BR sigaction () > +and testing whether the bit remains set in > +.IR oldact.sa_flags . > + > +Prior to Linux 5.x, unrecognized flag bits were preserved in > +.I oldact.sa_flags > +so this protocol on its own would not be sufficient to allow a > +userspace program to test for flag bit support if it needs to be > +compatible with kernel versions older than 5.x. Therefore, the > +.B SA_UNSUPPORTED > +flag bit was defined, which the kernel will always consider to be > +unknown. A userspace program that sets this flag bit in > +.I act.sa_flags > +and finds that it has been cleared in > +.I oldact.sa_flags > +in a subsequent call to > +.BR sigaction () > +may trust that any other unknown flag bits have been cleared. Is this a bit too much rationale? I don't think we have to justify the choice of design here -- describing the design and hinting at the reason for it seems enough. If you go with my suggestions for the SA_UNSUPPORTED section, then we can probably thin out some duplicate details here. > + > +A reasonably modern program may trust that the flags What does "reasonably modern" mean? > +.BR SA_NOCLDSTOP , > +.BR SA_NOCLDWAIT , > +.BR SA_SIGINFO , > +.BR SA_ONSTACK , > +.BR SA_RESTART , > +.BR SA_NODEFER , > +.B SA_RESETHAND > +and, if defined by the architecture, > +.B SA_RESTORER > +are supported by the kernel, without relying on the flag bit support > +detection protocol, since these flags have all been supported > +since Linux 2.6. I think it's best just to leave these unspecified, as today. Software seems to get on just fine without SA_UNSUPPORTED for these. Also, can we have a code example here? The description may leave people scratching their heads a bit, so I think a basic implementation would help. [...] Cheers ---Dave