Re: sigaction, SA_SIGINFO, and SIG_IGN

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Bruno Haible on 6/17/2008 8:12 PM:
|
| Eric Blake wrote:
|>
|> |>       if (sigaction (fatal_signals[i], NULL, &action) >= 0
|> |> +              && (action.sa_flags & SA_SIGINFO) == 0
|> |>           && action.sa_handler == SIG_IGN)
|> |>         fatal_signals[i] = -1;
|> |>     }
|>
|> Pre-existing bug.  POSIX states that sa_handler and sa_siginfo may, but
|> not must, occupy overlapping space, and that sa_handler must not be
|> accessed if sa_flags includes SA_SIGINFO.  Therefore, it is conceivable to
|> have an implementation where action.sa_handler is garbage and happens to
|> be SIG_IGN, because only action.sa_siginfo was valid to access.  (In
|> reality, all implementations I am aware of, including my replacement, use
|> a union and let the two function definitions overlap, so this is more of a
|> theoretical bug ...
|>
| What if someone actually installs a handler with SA_SIGINFO and SIG_IGN?

In theory, they can't, since SIG_IGN is typed differently than
sa_sigaction, but sa_sigaction is the only field that can be used when
SA_SIGINFO is set.  I'm adding the Austin Group, in case anyone else wants
to chime in.

| The kernel will not deliver such signals. I.e. the right way to fix the code
| would be
|
|     if (sigaction (fatal_signals[i], NULL, &action) >= 0
|         && (action.sa_flags & SA_SIGINFO
|             ? (void *) action.sa_sigaction == (void *) SIG_IGN
|             : action.sa_handler == SIG_IGN))

Not portable, and by your own arguments, not necessary.  C89 and C99
requires this to trigger a compilation warning, since SIG_IGN is typed as
a function pointer, which is not necessarily compatible with void *
(although POSIX 200x adds the restriction that void* must be compatible
with all function pointers).

|
| [Note that you cannot cast a function pointer into a different function
| pointer. gcc generates code equivalent to an abort() sometimes if you do
that.]

Casting between function pointer types is different than casting between
function pointer and void*.  C99 states this:

"A pointer to a function of one type may be converted to a pointer to a
function of another type and back again; the result shall compare equal to
the original pointer. If a converted pointer is used to call a function
whose type is not compatible with the pointed-to type, the behavior is
undefined."

gcc's abort() is acceptable if the user attempted to invoke the function
via the wrong type, but NOT on just casting it.

|
|> sa_handler must not be accessed if sa_flags includes SA_SIGINFO.
|
| The wording is not as clear as you might think.
|
| POSIX is not correct about the type of SIG_DFL and SIG_IGN:
| In <http://www.opengroup.org/susv3/basedefs/signal.h.html> it says
|   "...  constants, each of which expands to a distinct constant
expression of
|    the type  void (*)(int) ...: SIG_DFL SIG_IGN"
| but also
|   "void (*)(int, siginfo_t *, void *) sa_sigaction
|    Pointer to signal handler function or one of the macros SIG_IGN or
SIG_DFL."
| But sa_sigaction cannot be SIG_IGN or SIG_DFL because they are of different
| types!

I think you misquoted that.  In both 2001 (you gave the URL) and draft 5.1
of 200x, the description of struct sigaction only mention SIG_IGN and
SIG_DFL in relation to sa_handler, while the test for sa_sigaction states
only "Pointer to a signal-catching function".

But in researching this, I noticed that sigaction now has a rather big bug
introduced in the 200x draft.  In the process of moving
real-time-extension and some XSI functionality into base, line 60982 of
draft 5.1 now states:

"The sigaction( ) function shall fail if: [ENOTSUP] The SA_SIGINFO bit
flag is set in the sa_flags field of the sigaction structure."

In POSIX 2001, this was meant for the case where sigaction was supported
but SA_SIGINFO was not; but now that 200x requires SA_SIGINFO in Base,
this sentence taken literally means that you can't use SA_SIGINFO without
causing sigaction to fail with ENOTSUP!  What was intended was probably
removing the ENOTSUP clause altogether.

Furthermore, it adds line 60986:

"In addition, the sigaction( ) function may fail if the SA_SIGINFO flag is
set in the sa_flags field of the sigaction structure for a signal not in
the range SIGRTMIN to SIGRTMAX"

but without designation of which errno to use, and which seems to be in
conflict with sigqueue which has no similar restriction on SA_SIGINFO only
being used on SIGRT* signals.

|
| Also in <file:/other/www/www.opengroup.org/susv3/functions/sigaction.html>
| the sentence
|   "If the SA_SIGINFO flag is set in the sa_flags field, and the
implementation
|    supports the Realtime Signals Extension option or the XSI Extension
option,
|    the sa_sigaction field specifies a signal-catching function."
| indicates that the sa_sigaction field can never be SIG_DFL or SIG_IGN. Hence
| you always need to test sa_handler for that.
|
| According to this reading of POSIX, one is not allowed to use SA_SIGINFO
with
| SIG_DFL or SIG_IGN; in this case, the existing code in lib/fatal-signal.c is
| plain correct.

You missed my key point: both 2001 and 200x state:

"The storage occupied by sa_handler and sa_sigaction may overlap, and a
conforming application shall not use both simultaneously."

In other words, checking whether sa_handler is SIG_IGN is not possible
until you have first verified that sa_handler is accessible, and
sa_sigaction will never be SIG_IGN.  So I interpreted your quotes exactly
the way you did, which is why I added the check of SA_SIGACTION before
dereferencing sa_handler.

|
| In summary, this is too much theoretical head-ache for something which works
| in practice - since sa_handler and sa_sigaction share the same memory
location.
| You can put in a comment if you like. But no need to change code that works
| everywhere.

Does anyone know of a system where sa_handler and sa_sigaction do not
overlap, or where the kernel sets a signal handler to SIG_DFL or SIG_IGN
but accidentally sets the SA_SIGINFO flag in the return parameter of
sigaction?

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkhYeY8ACgkQ84KuGfSFAYDw0QCgkg2WfnEnr9y/GW/L/aS3TBAI
xZwAoL1AqtjkSDC8qrecwfY7WAGpVUDc
=S/5b
-----END PGP SIGNATURE-----