JonathonReinhart / staticx

Create static executable from dynamic executable
https://staticx.readthedocs.io/
Other
310 stars 34 forks source link

Request: forward all (well, most) signals to child process #240

Open brettdh opened 1 year ago

brettdh commented 1 year ago

I recently upgraded a Pyinstaller-built program to use Python 3.11. This resulted in the binary using a newer version of glibc than the target platform supported, which led me to discover staticx, without which this effort would be rather doomed. So first things first - thank you for building this. πŸ™‚

Back when I first started using Pyinstaller, I noticed that most signals were not being forwarded to the child process. In particular, my program uses a somewhat arbitrary signal number to control the logging level of the child process. A more commonly-used example would be SIGHUP, which is often used to ask a daemon to reload its configuration from disk.

I made a change to the Pyinstaller bootloader back in 2018 whereby it forwards all signals (except SIGCLD/SIGCHLD, because they're special) to the child process. I think something similar would work for staticx's (grammar...? πŸ˜… ) bootloader, but I haven't had a chance to try it out yet.

I might get some time to put together a PR for this, but I wanted to first open this issue so as to:

  1. get your feedback on the idea; and
  2. give you the chance to maybe get to it before I do 😁

Thanks again!

JonathonReinhart commented 1 year ago

Ah, you implemented that change to the PyInstaller bootloader after it had already influenced the staticx bootloader (167a5856b6085480f37e67b8573c53b56ed0ffa9). No wonder it's not there! πŸ˜„

I agree, all signals should be forwarded, not just termination signals, with the exception of SIGCHLD as you indicate, and SIGKILL and SIGSTOP, since according to signal(7):

The signals SIGKILL and SIGSTOP cannot be caught, blocked, or ignored.

I also think there's a mistake in pyinstaller/pyinstaller@2ddb80d89b699fcf65a092c3d2bede164882cdaf in the bounds of the for() loop: It should start at 1, not 0. And the upper bound is technically correct, but it is a bit misleading: I would use num_signals = 64, and use i <= num_signals.

I'm experimenting with an implementation now, but I'm seeing some strange differences between signal() and sigaction() on musl vs glibc... stay tuned.

JonathonReinhart commented 1 year ago

I've posted a draft PR in #242.

The "strange differences" I was referring to were the fact that SIGRTMIN starts at 35 on musl-libc (32-34 are reserved for pthreads implementation I believe), and thus sigaction() was failing.

I'm still not sure we want to forward all signals 1-31 with SIGCHLD, SIGKILL, and SIGSTOP in a denylist. If you read all of the signals listed on signal(7), I think there are plenty of others that we probably don't want to forward, too. Specifically ones that originate from hardware exceptions like SIGSEGV, SIGILL, SIGBUS, SIGFPE, etc.

I think we might consider instead an allowlist approach, including e.g.

brettdh commented 1 year ago

So long as the allow list is configurable when running staticx (with some reasonable defaults as you listed), that sounds just fine to me. Thanks for looking into it!

brettdh commented 1 year ago

In my original case, I was wrapping a legacy application which arbitrarily used signal 42. (A lot has changed since 2018, though, and I could change this to something else now if needed.)