Skip to content

fix(emerg-shutdown): close leaked input-device fds and swallow SIGSYS#370

Open
maybebyte wants to merge 2 commits into
Kicksecure:masterfrom
maybebyte:fix/emerg-shutdown-fd-leak-sigsys
Open

fix(emerg-shutdown): close leaked input-device fds and swallow SIGSYS#370
maybebyte wants to merge 2 commits into
Kicksecure:masterfrom
maybebyte:fix/emerg-shutdown-fd-leak-sigsys

Conversation

@maybebyte

Copy link
Copy Markdown
Contributor

Summary

Two independent, low-severity correctness fixes in the emerg-shutdown
panic-key monitor (the statically-linked C daemon that watches input devices
and triggers emergency shutdown): a file-descriptor leak during device
enumeration, and a signal-ignore loop that misses SIGSYS.

Changes

  • Close leaked input-device fds. Root cause: the device enumeration loop
    opens a descriptor for every /dev/input/eventN but only stores it for
    devices that support the panic-key combo. The two early-continue paths —
    non-key devices and keyboards lacking the panic keys — returned without
    closing tmp_fd, leaking one descriptor per non-qualifying device. The
    leak is bounded and one-time (the loop runs once at startup, not in the
    poll loop), but the fds are held for the daemon's lifetime and never
    reclaimed — the daemon never execs, so O_CLOEXEC does not cover them.
  • Swallow SIGSYS in the signal-ignore loop. max_sig_num is 31, but the
    SIG_IGN loop used a strict <, covering signals 1–30 and never touching
    signal 31 (SIGSYS on Linux). The real-time loop starts at SIGRTMIN, so
    SIGSYS was ignored by neither and kept its default terminate disposition,
    contrary to the "Swallow all signals that we can." intent. Changed < to
    <=; the loop still stops before the glibc-reserved signals 32/33.

Testing

  • Built via the project's own compile-emerg-shutdown after each commit —
    full hardening set (-Wall -Wextra -Werror=implicit -Wshadow … -static),
    clean build, valid ELF, no warnings-as-errors. -Werror=implicit confirms
    close() is properly declared (<unistd.h> is already included).
  • Signal boundaries are platform-checkable: on glibc/Linux signal 31 is
    SIGSYS and is ignorable (sigaction(SIGSYS, SIG_IGN) returns 0), while 32
    and 33 return EINVAL, so <= correctly stops at 31.

Notes for reviewers

  • Honest severity: both are low. The fd leak causes no exhaustion in practice
    (a handful of fds, once, well under RLIMIT_NOFILE); the SIGSYS change is a
    robustness/intent-alignment fix, not a hardening gain — SIGKILL and SIGSTOP
    are unblockable anyway, so anyone able to send SIGSYS can already kill the
    monitor. Its value is surviving an internally generated SIGSYS (e.g. a bad
    syscall).
  • Split into two commits so each can be accepted independently.

The input-device enumeration loop opens a descriptor for every
/dev/input/eventN but only stores it for devices that support the
panic-key combo. The two early-continue paths -- non-key devices and
keyboards lacking the panic keys -- returned without closing tmp_fd,
leaking one descriptor per non-qualifying device.

The leak is bounded and one-time (the loop runs once at startup, not in
the poll loop) so impact is low, but the descriptors are held for the
whole process lifetime and never reclaimed -- the daemon never execs,
so O_CLOEXEC does not cover them. Close tmp_fd before both continues.
max_sig_num is 31, but the SIG_IGN loop used a strict "<" bound, so it
covered signals 1..30 and never touched signal 31 (SIGSYS on Linux).
The real-time loop starts at SIGRTMIN, leaving SIGSYS covered by neither
loop, so it kept its default terminate disposition despite the "Swallow
all signals that we can." intent.

Use "<=" so the loop reaches 31; SIGSYS is ignorable, and the bound
still stops before the glibc-reserved signals 32 and 33. This is a
robustness fix, not a hardening gain: SIGKILL and SIGSTOP are unblockable
anyway, so an actor able to send SIGSYS can already kill the monitor --
the value is surviving an internally generated SIGSYS (e.g. a bad
syscall).
@maybebyte

Copy link
Copy Markdown
Contributor Author

AI assisted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant