Skip to content

Make exec image inspection a persistent monitor - #223

Merged
yunwei37 merged 4 commits into
mainfrom
feat/exec-image-monitor
Jul 22, 2026
Merged

Make exec image inspection a persistent monitor#223
yunwei37 merged 4 commits into
mainfrom
feat/exec-image-monitor

Conversation

@yunwei37

@yunwei37 yunwei37 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

  • keep PR scope limited to tutorial 54 only; the practical tutorial-design skill changes landed separately in Make tutorials 50–53 safer to operate #225
  • turn tutorial 54 from a child-process timeout harness into a persistent system-wide exec monitor
  • track concurrent exec callbacks per task and shut down by stopping admission, waiting for completed work, draining events, and reporting health counters
  • test explicit readiness, 16 concurrent execs, re-exec, signal cleanup, CLI failures, ELF decoding, and deferred file reads

Validation

  • host build: make -j2
  • KVM runtime: make test on x86_64 Linux 7.2.0-rc4
  • exact no-argument public workflow with an independent /bin/true workload and SIGTERM cleanup
  • bilingual exact-source checks for all three core source files

Runtime result: 18 matched, 18 scheduled, 18 callbacks, 18 completed, 18 delivered; zero schedule, drop, or cleanup errors.

This PR is intentionally left open for the next tutorial 54 revision pass.

@yunwei37

Copy link
Copy Markdown
Member Author

Dedicated pushed-head review — exact SHA a54f425cd1a8a8053fa0332a4d08dca0977d7f08

Verdict: approve; no blockers.

  • Shutdown order is consistent: detach/stop admission → wait for completed callbacks → drain → report → destroy.
  • The BPF helper split preserves dynptr discard, referenced-file release, pending-map deletion, and all counter semantics; the exact-head KVM run passed with 18 matched/scheduled/callback/completed/events and zero schedule, drop, or cleanup errors.
  • CodeFactor is now successful.
  • Code growth is reasonable: production +174/-334 (net -160), tests +131/-138 (net -7), overall textual diff net -487. The focused helpers add only 12 BPF lines versus the previous head and make ownership auditable.
  • Scope check passed: changes remain limited to the requested persistent tutorial-54 monitor, lifecycle/concurrency correctness, practical-use gates in the topic/design skills, and supporting docs/tests.

Non-blocking follow-ups: the test does not deterministically force shutdown with a callback still pending; the approximately one-second shutdown grace period may be short for very slow storage.

@yunwei37

yunwei37 commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Independent exact-head review — exact SHA a54f425cd1a8a8053fa0332a4d08dca0977d7f08

Verdict: approved; no blockers.

The separate read-only review confirmed:

  • per-exec HASH state and all success/error deletion paths are correct;
  • detach-before-drain plus scheduled == completed closes admission and shutdown races;
  • the CodeFactor-driven helper split preserves lifecycle semantics and materially improves readability;
  • the persistent CLI, concurrent readiness/signal tests, bilingual docs, and practical-use skill gates match the requested behavior;
  • code growth is justified and scope remains limited to tutorial 54, supporting tests/docs/diagram, and the two requested skills; tutorials 50–53 were not mutated.

AI-generated response; a maintainer will review and follow up later

@yunwei37

Copy link
Copy Markdown
Member Author

Exact-head repository review — 084d98f9cdba6dc92f979f9227cf2a8698b4a392

Verdict: approve; no blocking findings.

  • Rebase is exact against origin/main@b0d2c80; git diff --check, generated documentation, CodeFactor (zero issues), bilingual source-block checks, and the relevant host/KVM tests pass.
  • Tutorial 52 now decodes raw kernel dev_t correctly in BPF (major = dev >> 20, minor = dev & ((1<<20)-1)). Replacing one u64 with adjacent u32 fields preserves ABI layout, and the runtime test compares regular-file major:minor:inode against stat().
  • 50 persistent TCX lifecycle, 51 exact 4-tuple destructive safety, 53 fixed-path qdisc diagnostics/recovery, and 54 LSM-backed persistent monitoring/shutdown remain correct and in scope.
  • Growth audit: production net +88, tests net +169, skills net +33, docs net -2002. The growth is proportionate to safety/lifecycle tests; there is no unnecessary public CLI complexity or unrelated scope expansion.

One long CI build was still pending when this review completed; that is a terminal-green handoff gate, not a code-review finding.

@yunwei37

Copy link
Copy Markdown
Member Author

Independent external maintainer review — exact SHA 084d98f9cdba6dc92f979f9227cf2a8698b4a392

Verdict: approve; no blocking issues.

The independent read-only review re-audited the full PR and confirmed:

  • tutorial 52's raw kernel dev_t split is correct for Linux MINORBITS=20, preserves the event size/alignment, is verifier-safe on the null-inode path, and is meaningfully checked against the same regular file's stat() identity;
  • tutorial 50 link lifetime/counters, tutorial 51 dry-run plus exact IPv4 4-tuple destroy gate, tutorial 53 refusal/recovery and cleanup order, and tutorial 54 per-exec pending state plus detach→drain→destroy shutdown are sound;
  • LSM is required for tutorial 54's reliable executable-file/path inspection because the required file/path kfuncs are LSM-only and a tracepoint cannot retain a borrowed bprm->file across asynchronous task work;
  • generated bilingual docs and complete kernel-side source are synchronized; long user-space source is appropriately linked/summarized;
  • total change is a net reduction (about +1934/-3645), with production net +88, tests net +169, skills net +33, and docs around net -2001; no scope bloat or unnecessary boilerplate was found.

Only optional robustness/cosmetic nits were noted (quiet-KVM assumptions in two tests and pre-existing title consistency); none gate this PR.

@yunwei37
yunwei37 force-pushed the feat/exec-image-monitor branch from 136af2d to e311dc7 Compare July 22, 2026 08:38
@yunwei37

yunwei37 commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Exact-head review — dd5a9d50102c4880069b43828841e81b80fd9cec

Verdict: no blocking findings.

  • The final diff is confined to the eight files under src/54-exec-image-inspector/; the persistent system-wide monitor remains the sole functional scope.
  • Detach stops admission before the bounded completed >= scheduled wait and final drain. File references, dynptrs, and pending-map entries are released on every reviewed success/error path, and per-exec pid_tgid keys preserve concurrent execution handling.
  • The English/Chinese documentation, shared headers, BPF source blocks, CLI/output contract, requirements, DOT source, and generated PNG are synchronized.
  • The simplification is material: production C/BPF/header changes are net -275 lines, tests net -57, bilingual documentation net -1,198, and the complete textual diff is +345/-1891. No unnecessary helper or fixture growth remains.
  • git diff --check passes, all 7 GitHub checks are green, the PR is mergeable, and there are no unresolved review threads.

The recorded KVM run remains 18 matched/scheduled/callbacks/completed/events with zero schedule, drop, or cleanup errors. The current CI only rebuilds tutorial 54 rather than repeating that KVM run; stronger forced pending-callback shutdown coverage can be a later test improvement and does not block this scoped simplification.

AI-generated response; a maintainer will review and follow up later

@yunwei37
yunwei37 merged commit 90e93e0 into main Jul 22, 2026
7 checks passed
@yunwei37
yunwei37 deleted the feat/exec-image-monitor branch July 22, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants