distrib-compat 0.1-5: Backport upstream killproc fixes from bitstreamout/killproc#1637
Open
dcasota wants to merge 1 commit intovmware:5.0from
Open
distrib-compat 0.1-5: Backport upstream killproc fixes from bitstreamout/killproc#1637dcasota wants to merge 1 commit intovmware:5.0from
dcasota wants to merge 1 commit intovmware:5.0from
Conversation
Backport critical and robustness improvements from Werner Fink's killproc v2.23 (https://github.com/bitstreamout/killproc): High priority (correctness): - COMM_LEN: fix process name truncation in swap_name() for names >15 chars - sig_forced: prevent unwanted SIGKILL escalation on explicit -SIGTERM - pipe2(): eliminate startproc fork/exec race condition with pipe-based sync Medium priority (robustness): - UID-based fallback: non-root users can check owned processes on EACCES - expandpath(): avoid lstat() hangs on unresponsive NFS mounts - O_CLOEXEC: prevent fd leak to child processes in isscript() - atexit(undo_proc): auto-unmount /proc if mounted by the tool - Fix uninitialized variable warning in waiton() Tested: Photon OS 5.0, GCC 12.2.0 - zero errors, zero warnings. Smoke tests pass (killproc -l, checkproc on running/missing binaries). Signed-off-by: Daniel Casota <daniel@casota.ch>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport critical bug fixes and robustness improvements from Werner Fink's upstream killproc v2.23 into
distrib-compat 0.1. This update bumps the release from0.1-4to0.1-5by adding a single new patch file (Patch1) while preserving the unmodified Source0 tarball and its SHA-512 checksum.Motivation
Photon OS's
distrib-compatpackage (providingstartproc,killproc,checkproc) is a simplified fork of Werner Fink's killproc suite, originally developed for SUSE/openSUSE. The upstream project has accumulated several important fixes since the fork point that address real-world failure scenarios:Source Provenance and Upstream Attribution
All backported code originates from Werner Fink's bitstreamout/killproc repository. The original source files carry the following copyright headers:
The
distrib-compatpackage was created by Ankit Jain (VMware) in 2021 as a simplified multicall-binary adaptation of Werner Fink's killproc suite for Photon OS compatibility.License Compliance
GPL-2.0-only AND GPL-2.0-or-laterlicense_concluded: GPL-2.0-or-later AND GPL-2.0-onlyThe backported code is licensed under GPL-2.0-or-later (per upstream source file headers), which is fully compatible with the existing
distrib-compatlicense declaration. All modifications are derivative works under the same GPL-2.0 terms. No license change is required.Changes
High Priority -- Correctness Fixes
COMM_LEN process name truncation (
libinit.h): AddedCOMM_LENconstant (15 chars) and rewroteswap_name()to usestrncat()with proper truncation. The Linux kernel truncates/proc/<pid>/statcomm field to 15 characters; without this fix,killproc/checkprocfail to match processes whose binary name exceeds 15 characters (e.g.,NetworkManager,systemd-resolved).sig_forcedflag (killproc.c): Added a boolean flag that prevents unwanted SIGKILL escalation when the user explicitly specifies a signal (e.g.,killproc -SIGTERM /path/to/daemon). Previously,killprocwould always escalate to SIGKILL after timeout, even when SIGTERM was explicitly requested.pipe2()parent-child synchronization (startproc.c): Replaced the previous fork/exec sequence with pipe-based synchronization usingpipe2(O_CLOEXEC)(withpipe()+fcntl()fallback). This eliminates a race condition where the parent would check/proc/<pid>/exebefore the child had completedexecve(), causing spurious "start of ... failed" errors, particularly under system load.Medium Priority -- Robustness Improvements
UID-based fallback (
libinit.c): Inpidof(),verify_pidfile(), andcheck_pids(), added fallback logic for non-root users whenreadlink(/proc/<pid>/exe)returnsEACCES. Falls back to stat-based name matching using the caller's UID, enabling process checks by service users without elevated privileges.expandpath()function (libinit.c,libinit.h): Added a 96-linerealpath()alternative that avoidslstat()calls which can hang indefinitely on unresponsive NFS mounts. Used inpidof(),verify_pidfile(), andcheck_pids()for safe path resolution.O_CLOEXECon script open (libinit.c): AddedO_CLOEXECflag to theopen()call inisscript()to prevent file descriptor leaks to child processes.atexit(undo_proc)cleanup (libinit.c): Added automatic/procunmount on exit if the tool had to mount it, preventing stale mounts.Bonus: Fixed uninitialized variable
pwarning inwaiton()(startproc.c) -- a pre-existing issue not related to the backport but resolved to achieve a warning-free build.Files Changed
SPECS/distrib-compat/distrib-compat.spec-- Release bump 4 -> 5, added Patch1, changelog entrySPECS/distrib-compat/distrib-compat-upstream-backports.patch-- New 580-line unified diff (Patch1)The patch modifies 4 source files within the tarball:
libinit.h-- COMM_LEN constant, swap_name() rewrite, expandpath() declarationlibinit.c-- expandpath(), UID fallback, O_CLOEXEC, atexit cleanupkillproc.c-- sig_forced flagstartproc.c-- pipe2() sync, waiton() fixTesting
Build Verification
Patch Application
Patch0(distrib-compat-gen-debuginfo.patch) andPatch1(distrib-compat-upstream-backports.patch) apply cleanly in sequence to an unmodified extraction ofdistrib-compat-0.1.tar.bz2Smoke Tests
killproc -lcheckproc -v /usr/bin/bashcheckproc -v /usr/bin/nonexistentls -la /sbin/{killproc,checkproc,startproc}proctoolsFork Verification
Build and tests were additionally verified using the dcasota/photon fork's 5.0 branch, confirming that the spec and patch files from the fork's
SPECS/distrib-compat/directory are identical tovmware/photon:5.0and that the changes apply cleanly.Reviewer Guidance
Suggested test scenarios for extended validation:
checkproc/killprocwith daemons whose binary names exceed 15 characters (e.g.,NetworkManager,systemd-resolved,systemd-timesyncd)killproc -SIGTERM /path/to/daemondoes not escalate to SIGKILLcheckprocas a non-root user against a process owned by that user/usror/opt