supervisor: watch ktimerd and edr_daemon, and restore priority on restart - #129
Merged
Conversation
…tart
supervisor_watch() covered knetd only. ktimerd and the EDR daemon were
unsupervised, and neither failure is visible: timer_softirq_run() drives the
TCP timers, the DHCP renewal, the EDR periodic hooks and the CSPRNG reseed, so
a dead ktimerd degrades all four silently. Capacity raised 4 -> 8.
Found while wiring it: supervisor_restart() never restored priority.
task_create_kernel() assigns PRIORITY_NORMAL unconditionally, so a restarted
task came back demoted. This was invisible while knetd was the only watched
task -- knetd IS PRIORITY_NORMAL, so restoring its priority and failing to
restore it produce identical output. Both newly-watched tasks are
PRIORITY_HIGH, so the latent bug would have started biting immediately: the
task is alive and the restart counter rises, so every status surface reports a
healthy recovery, and the only symptom is the timer bottom-half running behind
interactive work.
Fixed by recording the priority in supervisor_watch() and restoring it in
supervisor_restart() before scheduler_add_task(). Set inside the restart
function rather than at the call site, for the same reason the rate limit lives
there: so no future restart path can skip it.
edr_daemon_main() loses `static` (the supervisor stores a void(*)(void));
kernel.c still starts the daemon via edr_daemon_start(). It is watched after
edr_daemon_start() returns rather than beside knetd/ktimerd, because the daemon
does not exist until then.
verify-supervisor.sh step 6: was a FALSE PASS, now grades.
The new leg indexed the last sample with "${KT_PRI_SAMPLES[-1]}". macOS ships
bash 3.2, which has no negative array subscripting, and under `set -u` that
raises "bad array subscript" / "unbound variable" rather than yielding empty.
That aborted the two assignments, leaving the operands unset; the `-ne`
comparisons then failed on stderr WITHOUT stopping the script, so the step
printed its "OK" line and the harness printed RESULT: PASS while grading
nothing.
The kernel was correct throughout -- the serial log showed ktimerd restarting
at priority 3 as intended -- so the only evidence was a stderr line nobody read.
That is the exact false-pass shape this suite exists to prevent.
Fixed by computing the last index explicitly and guarding the parse: all four
fields must be non-empty and numeric before any is compared, because `-ne`
treats an empty operand as a syntax error and CONTINUES.
Negative-controlled against the real serial log, since a fix that is not
falsified is how the false pass shipped:
- simulated demotion (post pri 3->2) -> FAIL "DEMOTED 3 -> 2"
- ktimerd never restarted (same PID) -> FAIL "not restarted (vacuous)"
- broken `ps -l` column layout -> FAIL "expected 2 rows, got 1"
- wrong baseline (pre pri 3->2) -> FAIL "baseline not 3"
- unmodified correct-kernel log -> OK
Fault injection uses ktimerd, not knetd, for the reason above: knetd's
PRIORITY_NORMAL cannot witness a demotion. The victim clears its OWN
CAP_UNKILLABLE and calls task_terminate(self_pid) -- not task_exit(), which is
inert for scheduler-run tasks -- so the capability check itself stays unmodified.
Verified: make clean + rebuild 0 warnings under -Werror; full
verify-supervisor.sh run PASSes all of steps 0-6, with step 6 reporting
pre PID=17356 pri=3 -> post PID=47438 pri=3 (restart proven by the PID change,
priority proven unchanged).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CEkhAhgTxbE5TgifyYf8v4
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.
Reopens #123, which GitHub auto-closed when its base branch
fix/double-fault-task-gatewas deleted on merging #122. Same commit, rebased ontomain(clean, no conflicts) — the content was never onmain, so this is not a duplicate.Original description follows.
Extends
supervisor_watch()to coverktimerdandedr_daemon, and restores task priority on restart.knetdwas the only supervised daemon.ktimerdandedr_daemonare created the same way and are just as capable of dying, but nothing noticed — a dead EDR daemon leaves the status surfaces reporting a healthy security posture, which is the status-surface lie class again: ask what the line would print if the mechanism were absent.Restart also used to leave the restarted task at default priority. That silently demotes a daemon that was created with an elevated one, so a daemon that has been restarted behaves differently from one that never died — the kind of difference that only shows up under the load that caused the restart.
Both changes go through the existing
supervisor_restart(), so the rate limit that lives inside it applies unchanged; no second restart path is introduced.Verification
make -j8 kernel.elfwarning-clean under-Werrorafter the rebase.verify-supervisor.shcovers the give-up path (step 5) and the pinned-counter assertion (5c).Note for review:
task_create_kernel()grantsCAP_ALL, which includesCAP_UNKILLABLE— so a test that plans tokillone of these daemons is grading a task that never died. The harness kills via the countdown (killknetd 8) for that reason.