Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8293466

libjsig should ignore non-modifying sigaction calls

    XMLWordPrintable

Details

    • b16

    Backports

      Description

        Found during code review of JDK-8292695.

        We have two bugs in libjsig when we install hotspot signal handlers. Relevant code in libjsig:

        ```
        int sigaction(int sig, const struct sigaction *act, struct sigaction *oact) {
        <snip>

          sigused = sigismember(&jvmsigs, sig);
          if (jvm_signal_installed && sigused) {
            /* jvm has installed its signal handler for this signal. */
            /* Save the handler. Don't really install it. */
            if (oact != NULL) {
              *oact = sact[sig];
            }
            if (act != NULL) {
              sact[sig] = *act;
            }

            signal_unlock();
            return 0;
          } else if (jvm_signal_installing) {
            /* jvm is installing its signal handlers. Install the new
             * handlers and save the old ones. */
            res = call_os_sigaction(sig, act, &oldAct);
            sact[sig] = oldAct;
            if (oact != NULL) {
              *oact = oldAct;
            }

            /* Record the signals used by jvm. */
            sigaddset(&jvmsigs, sig);

            signal_unlock();
            return res;
          }
        <snip>
        }
        ```

        Bug 1: we change state even if the sigaction call failed
        Bug 2: we change state even if the sigaction call was a non-modifying one (act == NULL)

        The latter is usually no problem since hotspot always calls `sigaction()` in pairs when installing a signal: first with NULL to get the old handler, then with the real handler. But this is not always true. If `AllowUserSignalHandlers` is set, and we find a custom handler is present, we will not override it:

        ```
        void set_signal_handler(int sig, bool do_check = true) {
          // Check for overwrite.
          struct sigaction oldAct;
          sigaction(sig, (struct sigaction*)NULL, &oldAct); <<<<< first sigaction call, libjsig now remembers signal as set

          // Query the current signal handler. Needs to be a separate operation
          // from installing a new handler since we need to honor AllowUserSignalHandlers.
          void* oldhand = get_signal_handler(&oldAct);
          if (!HANDLER_IS_IGN_OR_DFL(oldhand) &&
              !HANDLER_IS(oldhand, javaSignalHandler)) {
            if (AllowUserSignalHandlers) {
              // Do not overwrite; user takes responsibility to forward to us.
              return;
        ```

        That means:
        - we still have the original custom handler in place
        - but we already called sigaction, albeit with NULL, but libjsig now assumes that hotspot installed a handler itself.

        The result is that any further attempts to change the signal handler, whether by hotspot or by user code, will be prevented by libjsig. Any further non-modifying sigaction calls will return the original - still installed - custom handler.

        Admittedly, the error is very exotic. Users would have to set AllowUserSignalHandlers and preload libjsig, and *then* attempt to modify signal handlers after JVM initialization. But it is confusing, and a potential source for other errors. In hotspot, nobody counts on a non-modifying sigaction query changing program state somewhere.

        This seems to be an old bug, I see it in at least JDK 8. Did not look further into the past than that.

        Attachments

          Issue Links

            Activity

              People

                stuefe Thomas Stuefe
                stuefe Thomas Stuefe
                Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                  Created:
                  Updated:
                  Resolved: