init scripts: match daemons by name, not by a stale pidfile - #2326
Merged
Conversation
PR Summary by QodoMatch init daemons independently of stale pidfiles
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1. Six daemon changes untested
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Follow-up to the S95majestic fix. Given -p, busybox start-stop-daemon inspects only the pid that file holds and never scans /proc, so the pidfile is the sole liveness oracle -- and it goes stale on its own. Measured on a Hi3516EV300 (busybox 1.36.1), a -S against a stale pidfile starts a second copy of a daemon that is already running, and -m then overwrites the file with the duplicate's pid. Worse, every stop() here passed -p with no -x, so it signalled whatever process had since reused that pid: pointed at an unrelated live process, it killed that process and left the daemon running. Nine scripts drop the pidfile and match on -x, which scans every process: S01syslogd, S49ntpd, S50mdnsd, S60crond, S60precision-time, S60siproxd, S89edge, S90matter and S97baresip. start is now idempotent, stop polls until the daemon is really gone before restart proceeds, and reload signals through the same matcher instead of killall. syslogd, ntpd and crond match by name rather than by absolute path, and that is deliberate: they are busybox applets, so /proc/PID/exe points at /bin/busybox and a path never matches. Measured: -x syslogd -> match -x /sbin/syslogd -> no match Two scripts are deliberately left on their pidfiles, with a comment saying why, because name matching would be actively harmful: S50dropbear -- dropbear forks a child per SSH session sharing the listener's name and executable, so -x matching would make stop and restart kill every live session, including the one running the command. Measured: -K -x dropbear matched pids 908 and 607, where 607 was the listener and 908 was the caller's own session. Its pidfile is also trustworthy, being written by dropbear itself rather than by -m. S96onvifserver -- DAEMON is "httpd", a second busybox httpd beside the WebUI's own (S50httpd), and the two differ only in their arguments, which start-stop-daemon cannot match on. Both still gain an -x guard on -K, so a reused pid can no longer be signalled by mistake. Nothing outside these scripts reads any of the pidfiles. Verified on a Hi3516EV300: for syslogd, ntpd and crond a stale pidfile no longer spawns a duplicate, stop/start/restart each leave exactly one daemon with argv[0] unchanged, syslogd still receives from logger, and a cold boot brings up every daemon exactly once with :22, :80 and :554 listening.
Review catch: the -x "$DAEMON" guard added to -K accepts any httpd, and the WebUI runs a second busybox httpd (S50httpd) that differs from this one only in its arguments. A stale ONVIF pidfile whose pid the WebUI had since reused would therefore stop the WebUI -- the very thing keeping the pidfile was supposed to prevent. start-stop-daemon cannot match on arguments, so match them here instead: scan for the httpd whose command line carries $HTTPD_CONFIG. That names our instance exactly and can never name the WebUI's, which fixes both halves of the defect rather than only the collateral-damage half. start skips when our instance is already up and drops any leftover pidfile before -S, so a stale pid can no longer refuse the start either. Also add the missing -q to S90matter's -S, which was letting busybox's own "is already running" line print through the status message. Verified on a Hi3516EV300 with two real busybox httpd instances running side by side: ONVIF starts alongside the WebUI, a second start is refused, and with the pidfile poisoned to name the WebUI's pid, stop terminated the ONVIF instance and left the WebUI running.
widgetii
force-pushed
the
initd-drop-pidfile-matching
branch
from
August 29, 2026 07:50
fde08cc to
15bdf0d
Compare
7 tasks
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
1 participant
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.
Closes #2325. Follow-up to #2324, which fixed the same defect in
S95majestic. Disjoint files, so the two can land in either order.The defect
Given
-p, busyboxstart-stop-daemon'sdo_procinit()reads only the pid that file holds and never scans/proc.-xis then just a filter on that single pid, so once the pidfile is stale the live daemon is invisible. Two consequences, both reproduced on a Hi3516EV300 (busybox 1.36.1):Duplicate daemons.
-Scan't see the running instance, so it starts another — and-moverwrites the pidfile with the duplicate's pid, so the state is self-perpetuating.Killing unrelated processes. Every
stop()here passed-pwith no-x, so it signalled whatever process now owned that pid:What changed
Nine scripts drop the pidfile and match on
-x, which scans every process:S01syslogd,S49ntpd,S50mdnsd,S60crond,S60precision-time,S60siproxd,S89edge,S90matter,S97baresip.startbecomes idempotent,stoppolls until the daemon is genuinely gone beforerestartproceeds, andreloadsignals through the same matcher instead ofkillall.S90matteralso loses itskill -0 "$(cat "$PIDFILE")"guard, which was the same bug in longhand — it reports a reused pid as a live daemon.Two things that would have broken a naive sweep
syslogd,ntpdandcrondmust match by name, not by absolute path. They are busybox applets, so/proc/PID/exepoints at/bin/busyboxand the majestic-style path form silently never matches — which would have spawned a second syslogd on every board, on every boot. Measured:Two scripts are deliberately NOT converted, each with a comment saying why, because name matching would be actively harmful:
S50dropbear— dropbear forks a child per SSH session, and every one shares the listener's name and executable.-xmatching would makestopandrestartkill every live SSH session, including the one running the command:Its pidfile is also the trustworthy kind: there is no
-b/-min that script, so dropbear daemonises itself and writes the file, rather than start-stop-daemon guessing at it.S96onvifserver—DAEMONishttpd, a second busybox httpd living beside the WebUI's own (S50httpd). The two are told apart only by their arguments, whichstart-stop-daemoncannot match on, so the pidfile stays as the only thing that distinguishes them.Both still gain an
-xguard on-K, which removes the dangerous half — a reused pid can no longer be signalled by mistake:Nothing outside these scripts reads any of the pidfiles. (
siproxd.confsetspid_file, but siproxd writes that itself — dropping-mremoves a competing writer rather than a reader.)Verification (Hi3516EV300 + IMX335, busybox 1.36.1, kernel 4.9.37)
Every changed file passes
dash -nandsh -non-device. The four daemons that actually run on this board were exercised live:start→ no duplicatestop→ gone, pidfile clearedstopwhen already stopped → rc=0start→ running,argv[0]unchangedrestart→ exactly one daemonPlus:
loggerstill reaches syslogd after a restart; dropbear's match set verified to be the listener alone; and a cold boot brings up syslogd, ntpd, crond, dropbear and majestic exactly once each, withargv[0]unchanged for all, the three converted pidfiles absent, dropbear's own pidfile present and correct, and:22,:80and:554listening.The remaining six daemons (mdnsd, siproxd, ptp4l, n3n-edge, matter-server, baresip) are not enabled on this board, so the real daemons could not be run. Their init scripts were exercised on the camera instead, against a small static ARM ELF that waits on
pause()and dies on the defaultSIGTERM, installed at each daemon's true install path. That gives a genuine/proc/PID/exeandargv[0], so the matching, duplicate-refusal and shutdown logic -- the only thing this PR changes -- runs exactly as it will in production. All six pass start, stale-pidfile-no-duplicate, restart-leaves-one, stop, and stop-when-already-stopped;reloaddelivers SIGHUP to the right pid.The observed
argv[0]confirms the intended split: mdnsd, siproxd, n3n-edge and baresip keep a bareargv[0], while ptp4l and matter-server keep the absoluteargv[0]they already had. Nothing observable changes for either group.What this does not prove is each daemon's own behaviour under restart -- whether siproxd re-binds cleanly, whether ptp4l re-acquires its grandmaster. Those paths are untouched here, but if per-daemon hardware evidence is wanted first, the six can be split into a separate PR and held until each package can be built and run on a board that enables it.
ONVIF: matching by argument, not by pidfile
Review caught that the
-x httpdguard accepts any httpd, so a stale ONVIF pidfile whose pid the WebUI had reused could still stop the WebUI -- the very thing keeping the pidfile was meant to prevent.start-stop-daemoncannot match on arguments, soS96onvifservernow matches them itself, scanning for the httpd whose command line carries$HTTPD_CONFIG. That names our instance exactly and never the WebUI's, fixing both halves of the defect for this script too.Verified with two real busybox httpd instances side by side on the camera: ONVIF starts alongside the WebUI, a second start is refused, and with the pidfile poisoned to name the WebUI's pid (1589),
stopterminated the ONVIF instance (1616) and left the WebUI running.