From 1151029603979d553c144dd6609bd0fdfc969737 Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 3 Sep 2026 14:35:55 -0500 Subject: [PATCH 1/3] network: stop polling for absent WiFi, and kick ntpd when an address arrives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold-boot time sync is slow, and on WiFi it is slow for two independent reasons that compound. Closes #146. 1. S40network burns a flat 20s on every WiFi boot. It runs a literal `ifup -a`, which processes EVERY `auto` stanza -- including `auto wlan1`. On the ordinary single-dongle box `iw dev wlan1 info` never succeeds, so the v9 pre-up poll (4cf2fc7 / ADR 0016, for async-registering mainline rtw88/rtw89) runs all 20 iterations waiting for a device that will never appear. Measured against an absent interface: 20s. rcS is serial, so S41dhcpcd, S49ntp, S50sshd and Main_MiSTer all queue behind it. docs/wifi-parity.md §9 already found this hazard and fixed it for the HOTPLUG caller -- that is why 70-persistent-net.rules targets %k instead of stock's `-a`. The boot path never got the same treatment. Fixed with a `pre-up [ -e /sys/class/net/$IFACE ]` guard ahead of the loop in each wlan stanza. It costs no coverage: a device that is merely late is brought up by its own "add" uevent via that same udev rule, which fires on udev's boot-time coldplug as well as on later insertion. The v9 loop stays behind the guard for the narrow case it was written for -- netdev present, nl80211 not yet ready. 2. ntpd starts before there is any DNS, and only its retry backoff gets it out. S49ntp launches `ntpd -g` at a fixed point in rcS whether or not a network exists; on WiFi, association plus DHCP normally completes well after S49, so there is no /etc/resolv.conf yet (20-resolv.conf writes it on BOUND). ntpd heals one half of that and not the other. The INTERFACE half is fine: HAVE_RTNETLINK is set (config.h:786), so an address appearing schedules an endpoint rescan (ntp_io.c:4874) and ntp_io.c:1980 -> ntp_peer.c:763 refresh_all_peerinterfaces reattaches peers within ~3s. The DNS half is not: for a `server ` line the peer is created ONLY in the DNS callback (ntp_config.c:4459-4463 -> peer_name_resolved), so with resolution failing NO PEERS EXIST for that rescan to reattach, and peer_xmit returns early on !peer->dstadr (ntp_proto.c:4157) regardless. The 301s endpt_scan_period is a red herring; it never bites us. So the clock waits on the DNS retry backoff alone. libntp/ntp_intres.c manage_dns_retry_interval walks 2-3-4-6-8-12-16-24-32- 48-64s -- config.h:1313 leaves IGNORE_DNS_ERRORS undefined, so DNSFLAGS is 0 and retmax is 64, not 1024 -- putting attempts at t = 2, 5, 9, 15, 23, 35, 51, 75, 107, 155, 219, 283 ... s after ntpd started, then every 64s. The wait is the next slot after the address arrives: a few seconds wired, 10-20s on a typical WiFi boot, 30s to minutes when the network turns up late. New dhcpcd hook 91-ntp-kick restarts ntpd once, when an address actually arrives, so it re-resolves against the resolv.conf that now exists. /etc/ntp.conf's iburst and S49ntp's -g then give correct time within ~10-15s. A dhcpcd hook and not an init script or a udev rule, for the reason 90-timezone's header already gives: the event we want is "this box now has an address", and dhcpcd publishes exactly that. A net "add" uevent fires when the netdev is CREATED -- before association, lease, route or resolv.conf -- so kicking a DNS-dependent daemon there reproduces the bug one layer down. Not sntp: it would reach correct time ~10s sooner, but BR2_PACKAGE_NTP_SNTP also installs Buildroot's S48sntp (package/ntp/ntp.mk:110-114), which runs sntp at boot one script BEFORE ntpd with no network, and would have to be suppressed. Not worth a package symbol plus a post-build prune. Not service_condcommand, which 50-ntp.conf uses: dhcpcd's detect_init finds no systemctl/rc-service/invoke-rc.d/service/sv here and falls through to its /etc/init.d branch, which tests `[ -x /etc/init.d/ntpd ]` -- ours is S49ntp -- and service_status then runs `$x/$1 status`, a verb S49ntp has no case for. Both fail silently. The hook calls the script directly. (This is also why the pinned 50-ntp.conf is inert here, alongside etc/dhcpcd.conf leaving `option ntp_servers` commented out per stock; init-parity.md now says so.) The two fixes belong together: removing the 20s stall moves S49ntp 20s EARLIER while association does not move, so the gap between ntpd starting and the lease landing grows and ntpd lands deeper in its backoff. Hook design, all asserted by the new harness: fires at most once per boot (mkdir /run/ntp-kick is the atomic test-and-set, on a tmpfs, so it re-arms each boot -- RENEW recurs for the life of the session and restarting ntpd each time would discard its accumulated clock discipline); never starts an ntpd that is not running, so a by-hand or /etc/default/ntpd disable stands; and the stamp is claimed LAST, after every gate, so a pass that bails out leaves the one kick available. That last property is what makes the wired-box ordering case correct -- S41dhcpcd blocks until the first lease, so the hook fires before S49ntp has started ntpd at all. Backgrounded subshell (S49ntp restart contains a sleep 1), sourced not executed: no shebang, no exec bit, no exit. A static-IP box never runs dhcpcd and never gets kicked. Accepted -- the same gap ADR 0025 accepts -- and ntpd's own backoff still converges there. Tests: scripts/test-ntp-kick.sh, 29 sandboxed cases, no build/board/network needed. Wired into ci-tests.sh twice, host shell and the target's own BusyBox ash under qemu-arm (both 29/29 here). Mutation-checked: dropping the once-per-boot gate fails 2 cases, dropping the liveness check fails 2 others. ci-tests.sh also asserts the hook and S49ntp ship, and that /etc/ntp.conf keeps iburst -- without it the restart converges in minutes, not seconds, so a package bump could quietly undo the point of this change. check-linux-img.sh's pinned dhcpcd hook set gains 91-ntp-kick. Docs: init-parity.md gains a 91-ntp-kick row and corrects the pinned-hook-set row to say 50-ntp.conf is inert; init-parity/wifi-parity's "exactly 11 added lines" claims for etc/network/interfaces are now 47, zero removals (recounted against c196569, which was byte-identical to stock); wifi-parity §9 records that %k fixed only the hotplug caller and the boot path needed its own guard. Not addressed, deliberately: the clock still reads 1970 from power-on until the first sync. fake-hwclock-style persistence to /media/fat is the better answer to that and belongs in its own change; #146 says so. Verify-on-hardware: boot with a slow-enumerating rtw88 dongle and confirm the udev path alone brings it up now that S40network no longer waits for it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014SBpBTbtBsFwjAgKCx5T9S --- .../rootfs-overlay/etc/network/interfaces | 52 +++- .../usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick | 139 +++++++++ docs/init-parity.md | 5 +- docs/wifi-parity.md | 20 +- scripts/check-linux-img.sh | 8 +- scripts/ci-tests.sh | 51 ++++ scripts/test-ntp-kick.sh | 265 ++++++++++++++++++ 7 files changed, 525 insertions(+), 15 deletions(-) create mode 100644 board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick create mode 100755 scripts/test-ntp-kick.sh diff --git a/board/mister/de10nano/rootfs-overlay/etc/network/interfaces b/board/mister/de10nano/rootfs-overlay/etc/network/interfaces index 476af296..1cbfe42e 100644 --- a/board/mister/de10nano/rootfs-overlay/etc/network/interfaces +++ b/board/mister/de10nano/rootfs-overlay/etc/network/interfaces @@ -1,13 +1,47 @@ # interface file auto-generated by buildroot # -# MiSTer deviation (v9): each wlan stanza gains a pre-up wait loop before -# launching wpa_supplicant. USB WiFi drivers that initialise asynchronously -# (notably mainline rtw88/rtw89) may not have registered the nl80211 interface -# by the time ifupdown reaches the stanza, so wpa_supplicant would fail with -# "interface not found" on a cold boot. The loop polls `iw dev $IFACE info` -# (which succeeds only once nl80211 knows the device) for up to 20s. It runs -# AFTER the wpa_supplicant.conf existence check, so a system with no WiFi -# configured aborts the stanza first and never waits. +# MiSTer deviation (v9, revised): each wlan stanza gains a device-presence +# guard plus a wait loop before launching wpa_supplicant. +# +# THE RACE THE LOOP EXISTS FOR (v9, commit 4cf2fc7 / ADR 0016). USB WiFi +# drivers that initialise asynchronously -- notably the mainline rtw88/rtw89 +# drivers ADR 0016 adopted in place of the out-of-tree morrownr forks -- may +# not have registered the nl80211 interface by the time ifupdown reaches the +# stanza, so wpa_supplicant fails with "interface not found" on a cold boot. +# The `iw dev $IFACE info` poll (which succeeds only once nl80211 knows the +# device) covers that window, for up to 20s. +# +# WHY THE GUARD IN FRONT OF IT. S40network runs `ifup -a`, which processes +# EVERY `auto` stanza -- including `auto wlan1` on a box with a single dongle. +# Unguarded, the poll then runs its full 20 iterations waiting for a device +# that will never appear: a measured 20s added to rcS on every WiFi boot, with +# S41dhcpcd, S49ntp, S50sshd and Main_MiSTer all serialised behind it. This is +# the same waste that made etc/udev/rules.d/70-persistent-net.rules target %k +# instead of stock's `-a` (docs/wifi-parity.md §9) -- that fix covered the +# hotplug path only, and this covers the boot path it left behind. +# +# The guard splits the three cases the way they actually differ: +# +# device absent the stanza aborts instantly. Nothing is lost by +# not waiting: the netdev's own "add" uevent brings +# it up whenever it does appear, via +# 70-persistent-net.rules -> etc/wifi-hotplug.sh, +# and that rule fires on udev's boot-time coldplug +# (S10udevd's `udevadm trigger`) as well as on later +# insertion. Waiting here polls for an event we are +# already subscribed to. +# +# device present, but the v9 poll below still covers it. cfg80211 +# nl80211 not ready yet creates the netdev itself, so in practice this +# exits on the first iteration -- it is kept as a +# cheap safety net for the race v9 measured, not +# because it is expected to spin. +# +# device present and falls straight through, as before. +# ready +# +# Both lines run AFTER the wpa_supplicant.conf existence check, so a system +# with no WiFi configured aborts the stanza first and never reaches either. auto lo iface lo inet loopback @@ -15,6 +49,7 @@ iface lo inet loopback auto wlan0 iface wlan0 inet manual pre-up [ -f /media/fat/linux/wpa_supplicant.conf ] + pre-up [ -e /sys/class/net/$IFACE ] pre-up i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info >/dev/null 2>&1; do sleep 1; i=$((i+1)); done pre-up wpa_supplicant -s -B -P /run/wpa_supplicant.$IFACE.pid -i $IFACE -D nl80211,wext -c /media/fat/linux/wpa_supplicant.conf post_up sleep 2 @@ -23,6 +58,7 @@ iface wlan0 inet manual auto wlan1 iface wlan1 inet manual pre-up [ -f /media/fat/linux/wpa_supplicant.conf ] + pre-up [ -e /sys/class/net/$IFACE ] pre-up i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info >/dev/null 2>&1; do sleep 1; i=$((i+1)); done pre-up wpa_supplicant -s -B -P /run/wpa_supplicant.$IFACE.pid -i $IFACE -D nl80211,wext -c /media/fat/linux/wpa_supplicant.conf post_up sleep 2 diff --git a/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick b/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick new file mode 100644 index 00000000..1f81a1d6 --- /dev/null +++ b/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick @@ -0,0 +1,139 @@ +# Restart ntpd once, the first time this box gets an address and ntpd is +# already running. MiSTer -- sourced by dhcpcd-run-hooks, like every hook here. +# +# WHY THIS EXISTS. +# This board has no RTC, so on a cold boot the clock starts at the epoch and +# every timestamp -- save states, screenshots, the OSD clock, syslog, ntpd's own +# log lines -- is wrong until NTP lands. S49ntp starts `ntpd -g` at a fixed +# point in rcS, whether or not a network exists yet, and on the common path it +# does not: WiFi association plus DHCP usually completes well after S49. +# +# ntpd survives that in one respect and not in the other, and the difference is +# the whole reason for this file: +# +# the interface half heals itself. ntpd is built with HAVE_RTNETLINK +# (output/build/ntp-4.2.8p18/config.h:786), so an address appearing is a +# netlink event: ntp_io.c:4874 schedules an endpoint rescan UPDATE_GRACE +# seconds out, and ntp_io.c:1980 -> ntp_peer.c:763 refresh_all_peerinterfaces +# -> peer_refresh_interface reattaches peers to it within ~3s. Nothing to fix. +# +# the DNS half does not. /etc/ntp.conf names 0-3.pool.ntp.org, and for a +# `server ` line the peer is created ONLY in the DNS callback +# (ntp_config.c:4459-4463 -> peer_name_resolved). At S49 there is no +# /etc/resolv.conf yet -- 20-resolv.conf writes it in this very hook pass -- +# so resolution fails and NO PEERS EXIST. The rescan above then has nothing +# to reattach, and peer_xmit returns early on !peer->dstadr +# (ntp_proto.c:4157) regardless. +# +# So the clock waits on ntpd's DNS retry backoff, and nothing else. +# libntp/ntp_intres.c manage_dns_retry_interval walks 2-3-4-6-8-12-16-24-32-48-64 +# seconds; config.h:1313 leaves IGNORE_DNS_ERRORS undefined, so DNSFLAGS is 0 and +# retmax is 64, not 1024. Attempts therefore land at t = 2, 5, 9, 15, 23, 35, 51, +# 75, 107, 155, 219, 283 ... seconds after ntpd started, then every 64s. The wait +# is however long until the next slot after the address arrives: a few seconds on +# a wired box, 10-20s on a typical WiFi boot, and 30s to a couple of minutes when +# the network turns up late (slow AP, a cable plugged in after boot, or WiFi +# configured on a card that has already been booted once). +# +# A restart re-resolves immediately against the resolv.conf that now exists. +# /etc/ntp.conf carries `iburst` on all four pool lines, so ntpd gets its samples +# at ~2s intervals rather than at minpoll, and S49ntp's `-g` permits the one +# large step a no-RTC box needs -- correct time within roughly 10-15s of the kick. +# +# WHY A DHCP HOOK, AND NOT AN INIT SCRIPT OR A UDEV RULE. +# Same argument as 90-timezone's, which see: the event we want is "this box now +# has an address", and dhcpcd is what publishes it. An init script can only ask +# whether there is a network a few seconds into boot, which is the wrong question +# -- that is precisely the state that created this problem. A udev rule is wrong +# in the other direction: a net `add` uevent fires when the netdev is CREATED, +# before association, before a lease, before a route, before resolv.conf. Kicking +# a DNS-dependent daemon there would reproduce the bug one layer down. +# +# dhcpcd is already fully event-driven underneath this: it runs in master mode +# and learns about new interfaces from netlink (RTM_NEWLINK -> +# dhcpcd_handleinterface, src/if-linux.c:1137), so a dongle plugged in three +# boots later reaches this hook with no init involvement at all. +# +# WHY sntp IS NOT USED HERE. +# `sntp -Ss` would set the clock in about a second instead of ten, but +# BR2_PACKAGE_NTP_SNTP also installs Buildroot's own /etc/init.d/S48sntp +# (package/ntp/ntp.mk:110-114), which runs sntp at boot -- one script before +# ntpd, with no network -- and would have to be suppressed. Ten seconds is not +# worth a package symbol plus a post-build prune. +# +# WHY NOT service_condcommand, WHICH 50-ntp.conf USES. +# It cannot work on this image, for two independent reasons. dhcpcd's detect_init +# (dhcpcd-run-hooks) finds no systemctl/rc-service/invoke-rc.d/service/sv here and +# falls through to its /etc/init.d branch, which tests `[ -x /etc/init.d/ntpd ]` +# -- ours is S49ntp. And service_status runs `$x/$1 status`, a verb S49ntp has no +# case for. Both fail silently, returning 1. This calls the script directly. +# +# ONCE PER BOOT, AND ONLY FOR AN ntpd THAT IS ALREADY RUNNING. +# The mkdir is the atomic test-and-set, on a tmpfs, so it re-arms every boot and +# cannot be won twice: two interfaces can BOUND at the same moment, and RENEW +# fires again every few hours for the life of the session. Restarting ntpd on +# each renewal would throw away its accumulated clock discipline for no reason, +# which is a real cost rather than a theoretical one. +# +# The running check is what keeps this from starting a daemon nobody asked for -- +# if ntpd was disabled by hand, or via /etc/default/ntpd, that decision stands. +# It also handles the ordering case that looks like a bug and is not: on a WIRED +# box, S41dhcpcd blocks until the first lease is bound, so this hook fires DURING +# S41 -- before S49ntp has started ntpd at all. The check fails, the stamp is +# deliberately not claimed (it is taken last, after every gate), and rcS then +# starts ntpd a moment later with resolv.conf already in place, which is the +# outcome we wanted anyway. Nothing is lost and nothing needed kicking. +# +# A static-IP box configured only in /etc/network/interfaces never runs dhcpcd, +# so it never gets here. Accepted, and the same gap ADR 0025 accepts: ntpd's own +# backoff still converges there, just on its own schedule. +# +# DELAYS NOTHING. The work runs in a backgrounded subshell -- S49ntp's `restart` +# has a `sleep 1` in it, and a hook must not hold up dhcpcd's chain. Being +# sourced, this file also must not `exit`: that would end dhcpcd's whole hook run +# and take 20-resolv.conf and 30-hostname with it. The subshell answers both at +# once, and keeps every variable below out of dhcpcd's shell. +# +# The *6 reasons cannot fire on this kernel -- it is built without IPv6 +# (linux.config: "# CONFIG_IPV6 is not set") -- but they cost nothing and are +# correct the day that changes. + +case "$reason" in +BOUND|REBOOT|REBIND|RENEW|BOUND6|REBOOT6|REBIND6|RENEW6) + # if_up is compared as DATA, not executed. dhcpcd's own hooks write + # `if $if_up`, which works because dhcpcd sets it to the literal string + # true or false -- but this is sourced into dhcpcd's shell, where + # anything else in that variable would then run as a command there. + if [ "${if_up:-false}" = true ]; then + ( + NTP_INIT=/etc/init.d/S49ntp + # S49ntp passes `-p $PIDFILE` to ntpd, so ntpd itself writes + # this. /var/run is a usr-merge-era symlink to /run (a tmpfs), + # so the file cannot outlive a boot. If /etc/default/ntpd has + # moved PIDFILE, the check below simply fails and this hook does + # nothing -- the safe direction. + NTPD_PID=/var/run/ntpd.pid + # tmpfs, so the once-per-boot stamp re-arms on every boot. + STAMPDIR=/run/ntp-kick + + # Cheapest checks first; the stamp is claimed LAST, so a pass + # that bails out early leaves the one kick still available. + [ -x "$NTP_INIT" ] || exit 0 + [ -s "$NTPD_PID" ] || exit 0 + ntpd_pid="$(cat "$NTPD_PID" 2>/dev/null)" + case "$ntpd_pid" in + ''|*[!0-9]*) exit 0 ;; + esac + # A stale pidfile cannot survive the tmpfs, but a crashed ntpd + # within one boot can leave one. kill -0 is the liveness test. + kill -0 "$ntpd_pid" 2>/dev/null || exit 0 + + mkdir "$STAMPDIR" 2>/dev/null || exit 0 + + echo "ntpd: address acquired, restarting to re-resolve the NTP pool" + "$NTP_INIT" restart + ) & # NTP-KICK-BACKGROUND -- scripts/test-ntp-kick.sh keys on this + # marker to run the body synchronously; keep it if the line moves. + fi + ;; +esac diff --git a/docs/init-parity.md b/docs/init-parity.md index fc702bca..24d4a4c0 100644 --- a/docs/init-parity.md +++ b/docs/init-parity.md @@ -147,12 +147,13 @@ regardless, so the worst case is slow, never weak. | `/etc/fstab` | **adapted** (2 additive deviations) | All of stock's entries reproduced verbatim (this row said "identical / byte-for-byte, `diff` exit 0" and was stale — two tmpfs lines stock does not have were appended later: `tmpfs /var/cache/samba tmpfs mode=0755 0 0` per P3.6 / `docs/samba-parity.md` §3, asserted by `scripts/ci-tests.sh`, and `tmpfs /var/lib/seedrng tmpfs mode=0700 0 0` for BusyBox `S01seedrng`. `diff` against `work/imgroot/etc/fstab` now exits 1 with exactly those two lines plus a 6-line explanatory comment, and zero removals): ext4 `rw,noauto,noatime,nodiratime` root, tmpfs on `/tmp`, `/run`, `/dev/shm`, `/var/lib/samba`, `/var/db/dhcpcd`, plus `proc`/`devpts`/`sysfs`. | | `/etc/hostname` | **identical** | `MiSTer\n` (7 bytes), byte-for-byte match. | | `/etc/hosts` | **identical** | Byte-for-byte match (`127.0.1.1 MiSTer`). Buildroot's own finalize hook writes `127.0.1.1 buildroot` into `output/target/etc/hosts` **before** the overlay is copied (confirmed in the build log); the overlay copy runs later and wins — verified in the extracted image. | -| `/etc/network/interfaces` | **adapted (v9)** (1 additive deviation) | `lo` + `wlan0`/`wlan1` with the `wpa_supplicant -D nl80211,wext` pre-up hooks, all of stock's directives reproduced unchanged. No `eth0` stanza — matches stock exactly; wired ethernet is handled by dhcpcd's own default (manage-everything-not-explicitly-excluded) behavior, not ifupdown. **This row said "identical / byte-for-byte" until now and was stale:** P2.3 authored it byte-identical, but `4cf2fc7` (v9) added a 9-line header comment plus one `pre-up i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info >/dev/null 2>&1; do sleep 1; i=$((i+1)); done` line per `wlan` stanza, because USB WiFi drivers that register `nl80211` asynchronously (mainline `rtw88`/`rtw89`) otherwise lose the race and `wpa_supplicant` fails with "interface not found" on a cold boot. `diff` against `work/imgroot/etc/network/interfaces` now exits 1 with exactly those 11 added lines and zero removals (re-verified). See `docs/wifi-parity.md` §1 and §9 — §9's hotplug rule depends on that loop existing. | +| `/etc/network/interfaces` | **adapted (v9)** (1 additive deviation) | `lo` + `wlan0`/`wlan1` with the `wpa_supplicant -D nl80211,wext` pre-up hooks, all of stock's directives reproduced unchanged. No `eth0` stanza — matches stock exactly; wired ethernet is handled by dhcpcd's own default (manage-everything-not-explicitly-excluded) behavior, not ifupdown. **This row said "identical / byte-for-byte" until now and was stale:** P2.3 authored it byte-identical, but `4cf2fc7` (v9) added a header comment plus one `pre-up i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info >/dev/null 2>&1; do sleep 1; i=$((i+1)); done` line per `wlan` stanza, because USB WiFi drivers that register `nl80211` asynchronously (mainline `rtw88`/`rtw89`) otherwise lose the race and `wpa_supplicant` fails with "interface not found" on a cold boot. **Revised since:** each stanza also gained a `pre-up [ -e /sys/class/net/$IFACE ]` device-presence guard ahead of that loop, because `S40network`'s `ifup -a` processes *every* `auto` stanza — so on the ordinary single-dongle box the `wlan1` stanza polled the full 20 s for a device that never appears, serialising `S41dhcpcd`/`S49ntp`/`S50sshd`/Main_MiSTer behind it on every WiFi boot (measured: 20 s). A device that is merely late is brought up by its own `add` uevent via `70-persistent-net.rules` → `etc/wifi-hotplug.sh`, which fires on udev's boot-time coldplug as well as on later insertion, so the guard costs no coverage. `diff` against `work/imgroot/etc/network/interfaces` now exits 1 with exactly 47 added lines (a 43-line header comment plus the guard and the loop in each of the two `wlan` stanzas) and zero removals. See `docs/wifi-parity.md` §1 and §9. | | `/etc/dhcpcd.conf` | **identical** | Byte-for-byte match. The package's own default differs meaningfully (`#hostname`/`#clientid` instead of stock's enabled `hostname`/`clientid`, `duid` instead of stock's `#duid`, and is missing the `option rapid_commit` block) — all reverted to stock via the overlay. | | `/etc/inittab` | **adapted** (3 documented deviations) | Full stock shape reproduced (`::sysinit:/media/fat/MiSTer &`, `/etc/resync &`, `rcS`, shutdown sequence) with: **(1)** the remount-rw sysinit line kept **commented out**, exactly as stock has it — Buildroot's own skeleton default inittab ships this line **uncommented**, which would remount `/` rw at every sysinit and defeat the whole read-only-root design (ADR 0011); confirmed this project's own finalize hook tries to uncomment it too (see below) and is overridden by the overlay running last. **(2)** the serial console runs `ttyS0::respawn:/sbin/agetty --nohostname -L ttyS0 115200 vt100` — util-linux `agetty`, matching stock (whose inittab also uses `agetty --nohostname`), now that `BR2_PACKAGE_UTIL_LINUX_AGETTY` is enabled (see `docs/util-linux-parity.md`). It still targets `ttyS0` explicitly rather than stock's `console` alias, because this board's actual cmdline is `console=ttyS0,115200` (`docs/boot-chain.md`). agetty treats a numeric positional argument as the baud rate, so `ttyS0 115200` and `115200 ttyS0` are equivalent; the port-first order is kept for continuity with the previous BusyBox `getty` line. (Earlier revisions used BusyBox `getty` because `BR2_PACKAGE_UTIL_LINUX_BINARIES` was not selected — that constraint no longer holds; the overlapping BusyBox `getty` applet is now disabled so `agetty` is the console.) **(3)** `gpm` is invoked at `/usr/sbin/gpm`, not stock's `/sbin/gpm` — originally **a real bug caught during acceptance verification**: gpm is a real package (not a BusyBox applet, which lands under `/sbin` regardless of usr-merge), and when this row was first written (P2.3) the rootfs was **not** usr-merged — `/sbin` was a real directory, not `-> /usr/sbin` the way stock's `work/imgroot/sbin` is — so `/sbin/gpm` genuinely did not exist in the image. An inittab `sysinit` line is an **absolute path**, so unlike `/etc/profile`'s `PATH` it gets no shell search at all; it would have silently failed to spawn on every single boot. Caught by exhaustively grep'ing every absolute path referenced anywhere in the overlay against the actually-built image (see the task report) — first build had this wrong; fixed and rebuilt before acceptance. **Since P2.9 v2** (`7be9ee5`) set `BR2_ROOTFS_MERGED_USR=y`, that premise no longer holds: this build is now usr-merged like stock (verified: `output/target/sbin -> usr/sbin`), so `/sbin/gpm` *would* resolve today. **The deviation is kept anyway, deliberately.** Buildroot builds gpm with `--prefix=/usr`, so the binary's real path is `/usr/sbin/gpm` in *either* layout (gpm's own file list: `gpm,./usr/sbin/gpm`) — stock's binary is at that same physical path, and stock's inittab only reaches it through the `/sbin -> usr/sbin` compat symlink that exists solely because stock is usr-merged. So our path is correct in both layouts and stock's is correct in only one; ours survives an unmerge, stock's would break on one. Given P2.3 already got bitten by exactly that (the unmerged build had no `/sbin/gpm` at all), pointing at the real path rather than a merge-dependent alias is the more durable choice, even though a future unmerge is unlikely. This is an **accepted permanent deviation from stock**, not a cleanup waiting to happen. **Also dropped — later RESTORED, guarded (T3, 2026-07-27):** `loadkeys /etc/kbd.map` and `setfont` were dropped by P2.3 because this BusyBox build has no `loadkeys` applet at all and `CONFIG_SETFONT` is explicitly not set (still true — re-verified in `output/build/busybox-1.38.0/.config`; busybox's `loadkmap` applet is not a substitute, it reads binary bkeymap, not stock's text keymap); keeping either line verbatim would just fail every boot. T3 vendored stock's `etc/kbd.map` (it blanks the F12/Mute/Vol± keycodes Main_MiSTer consumes via evdev) and restored both lines wrapped in `[ -x /usr/bin/... ]` guards: with the parallel T5 task's `BR2_PACKAGE_KBD` (the same package stock's own loadkeys/setfont came from) the lines do stock's exact job, without it they are silent no-ops instead of boot errors. See the inittab's own note 3 and `docs/stock-reconciliation.md` §3c. `gpm -m /dev/input/mice -t imps2` itself **is** kept (just at the corrected path) — `BR2_PACKAGE_GPM=y` was deliberately selected in P2.1 for this, and the binary is present. | | `/etc/profile` | **adapted** (1 documented deviation) | Full stock content reproduced (PATH, `PS1='$(pwd)# '`, `EDITOR=/bin/vi`, `/etc/profile.d/*.sh` sourcing, `LC_ALL=en_US.UTF-8`, and critically the login-time `mount -o remount,rw /` — this is how `/` ever becomes writable at all, matching stock and ADR 0011's own description of the mechanism). **Deviation:** `PATH` gains an explicit `/bin:/sbin:` prefix stock's literal string doesn't have. Stock's rootfs is usr-merged (`work/imgroot`: `/bin -> usr/bin`, `/sbin -> usr/sbin`), so its `PATH="/usr/bin:/usr/sbin"` already covered `/bin`/`/sbin` for free. When this deviation was introduced (P2.3) *this* build was a plain (non-merged) skeleton layout, so omitting `/bin:/sbin` from `PATH` would have silently dropped most BusyBox applets from every interactive shell; changing `BR2_ROOTFS_MERGED_USR` was then out of P2.3's scope ("do NOT disturb ... the package set"), making the overlay `PATH` the correct-altitude fix. **Since P2.9 v2** (`7be9ee5`) set `BR2_ROOTFS_MERGED_USR=y` — the change that fixed the `/lib/security/pam_unix.so` SSH lockout — this build is usr-merged too, so the `/bin:/sbin:` prefix is now redundant. It is kept because it is harmless (the paths resolve to the same directories) and matches Buildroot's own skeleton default profile. The file's own header comment records the same history. | | `/usr/lib/dhcpcd/dhcpcd-hooks/90-timezone` | **added (divergence, [ADR 0025](decisions/0025-first-boot-timezone-autodetect.md))** | Stock has no equivalent, and that is the gap it closes: `/etc/localtime` points at `/media/fat/linux/timezone`, which **does not exist on a fresh card**, so glibc falls back to UTC silently and permanently. The first time dhcpcd brings an interface up with an address, this asks `ip-api.com` for the zone of the box's public IP and copies `/usr/share/zoneinfo/posix/` to that path — the *same* provider, destination and file format as the community `Scripts_MiSTer/timezone.sh` "Automatic" mode, so the two are interchangeable. No new package (`curl` and tzdata were already in the image). **Note this adds no init script**: an earlier revision had an `S48timezone` as well, but on a DHCP box the lease has usually not landed by S48, so it was near-redundant with this hook — and it carried a `/proc/net/route` check whose IPv6 arm silently matched the kernel's own `ip6_null_entry`. A static-IP box configured only in `/etc/network/interfaces` never runs dhcpcd and so never autodetects; accepted deliberately, since setting a static address is already a by-hand act. Properties worth stating because they are the design: the guess is spent **once, and only when it was actually made** (gated on the timezone file *and* on a `timezone.autodetect` stamp written only when a provider answered *with a zone name*, so neither being offline nor a captive portal's HTTP 200 burns it), it **never overwrites** a timezone anyone has already set (re-checked immediately before the write, not only at the gate), and it **delays nothing** — the body is a backgrounded subshell, which also keeps it from leaking a single variable or function into dhcpcd's shell. Sourced, not executed (`dhcpcd-run-hooks`: `. "$hook"`), hence no shebang, no exec bit, and no `exit` — an `exit` here would end dhcpcd's whole hook run and take `20-resolv.conf`/`30-hostname` with it. Zone names arrive off the network, so they are validated against the shipped zoneinfo before being used as a path; `scripts/test-timezone.sh` asserts each rejection, and mutation-checks the two that could otherwise pass vacuously. | -| `/usr/lib/dhcpcd/dhcpcd-hooks/` (the set) | **pinned (build-host independence)** | dhcpcd's `configure` chooses which hooks to install by probing the **build host** for `ntpd`/`chronyd`/`systemd-timesyncd`/`ypbind`, and Buildroot's `dhcpcd.mk` passes no `--with-hooks`, so the image inherited whatever daemons the build machine happened to have: a GitHub runner produced `50-ntp.conf`, a developer box with timesyncd produced `50-timesyncd.conf` — with a green build both times (found 2026-09-02 by diffing a CI image against a local one). `external.mk` now appends `--with-hooks=ntp.conf --with-eghooks=yp.conf` to `DHCPCD_CONFIG_OPTS`, which reproduces the canonical CI image exactly (`01-test`, `20-resolv.conf`, `30-hostname`, `50-ntp.conf`, plus our `90-timezone`; `50-yp.conf` stays an example under `/usr/share/dhcpcd/hooks`), and `scripts/check-linux-img.sh` asserts that exact set. `50-ntp.conf` is the right hook: we ship classic `ntpd` (stock parity) and it writes DHCP-offered servers into `/etc/ntp.conf`. Upstream-worthy: Buildroot's `dhcpcd.mk` should pass `--with-hooks` itself. | +| `/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick` | **added (divergence)** | Stock has no equivalent. The gap: this board has no RTC, so a cold boot starts at the epoch, and `S49ntp` launches `ntpd -g` at a fixed point in `rcS` whether or not a network exists — which on the common path it does not, since WiFi association plus DHCP normally completes well after S49. ntpd survives that in one half and not the other. The **interface** half heals itself: ntpd is built with `HAVE_RTNETLINK` (`config.h:786`), so an address appearing is a netlink event and `ntp_io.c:1980` → `ntp_peer.c:763` `refresh_all_peerinterfaces` reattaches peers within ~3 s. The **DNS** half does not: for a `server ` line the peer is created *only* in the DNS callback (`ntp_config.c:4459-4463` → `peer_name_resolved`), and at S49 there is no `/etc/resolv.conf` — `20-resolv.conf` writes it in this very hook pass — so resolution fails and **no peers exist at all** for the rescan to reattach. The clock then waits on ntpd's DNS retry backoff and nothing else: `libntp/ntp_intres.c` `manage_dns_retry_interval` walks 2-3-4-6-8-12-16-24-32-48-64 s (`config.h:1313` leaves `IGNORE_DNS_ERRORS` undefined, so `DNSFLAGS` is 0 and `retmax` is 64, not 1024), putting attempts at t = 2, 5, 9, 15, 23, 35, 51, 75, 107, 155, 219, 283 … s after ntpd started. Measured cost of the next-slot wait: a few seconds on a wired box, 10-20 s on a typical WiFi boot, 30 s to a couple of minutes when the network turns up late. This restarts ntpd once, when an address actually arrives, so it re-resolves against the `resolv.conf` that now exists; `/etc/ntp.conf`'s `iburst` and `S49ntp`'s `-g` then give correct time within ~10-15 s. **Not `sntp`**: `BR2_PACKAGE_NTP_SNTP` would also install Buildroot's `S48sntp` (`package/ntp/ntp.mk:110-114`), which runs sntp at boot one script *before* ntpd with no network, and would have to be suppressed — not worth ~10 s. **Not `service_condcommand`** (which `50-ntp.conf` uses): dhcpcd's `detect_init` finds no systemctl/rc-service/invoke-rc.d/service/sv here and falls through to its `/etc/init.d` branch, which tests `[ -x /etc/init.d/ntpd ]` — ours is `S49ntp` — and `service_status` then runs `$x/$1 status`, a verb `S49ntp` has no case for; both fail silently. The hook calls the script directly. **Not a udev rule**: a `net` `add` uevent fires when the netdev is *created*, before association, lease, route or `resolv.conf`, which would reproduce the bug one layer down. Design properties: fires **at most once per boot** (`mkdir /run/ntp-kick` is the atomic test-and-set, on a tmpfs, so it re-arms each boot — `RENEW` recurs for the life of the session and restarting ntpd each time would discard its accumulated clock discipline); **never starts an ntpd that is not running**, so a by-hand or `/etc/default/ntpd` disable stands; and the stamp is claimed **last**, after every gate, so a pass that bails out leaves the one kick available — which is what makes the wired-box ordering case correct, where `S41dhcpcd` blocks until the first lease and so fires this hook *before* `S49ntp` has started ntpd at all. Backgrounded subshell (`S49ntp restart` contains a `sleep 1`), sourced not executed — no shebang, no exec bit, no `exit`. A static-IP box never runs dhcpcd and never gets kicked; accepted, the same gap ADR 0025 accepts, and ntpd's own backoff still converges there. `scripts/test-ntp-kick.sh` asserts the behaviour in 29 sandboxed cases (run twice by `ci-tests.sh`: host shell, and the target's own BusyBox ash under qemu-arm), mutation-checked against dropping the once-per-boot gate and the liveness check. | +| `/usr/lib/dhcpcd/dhcpcd-hooks/` (the set) | **pinned (build-host independence)** | dhcpcd's `configure` chooses which hooks to install by probing the **build host** for `ntpd`/`chronyd`/`systemd-timesyncd`/`ypbind`, and Buildroot's `dhcpcd.mk` passes no `--with-hooks`, so the image inherited whatever daemons the build machine happened to have: a GitHub runner produced `50-ntp.conf`, a developer box with timesyncd produced `50-timesyncd.conf` — with a green build both times (found 2026-09-02 by diffing a CI image against a local one). `external.mk` now appends `--with-hooks=ntp.conf --with-eghooks=yp.conf` to `DHCPCD_CONFIG_OPTS`, which reproduces the canonical CI image exactly (`01-test`, `20-resolv.conf`, `30-hostname`, `50-ntp.conf`, plus our `90-timezone` and `91-ntp-kick`; `50-yp.conf` stays an example under `/usr/share/dhcpcd/hooks`), and `scripts/check-linux-img.sh` asserts that exact set. `50-ntp.conf` is the right hook to pin given we ship classic `ntpd` (stock parity), though it is **inert in this image**: it merges DHCP-offered servers into `/etc/ntp.conf`, and `etc/dhcpcd.conf` leaves `option ntp_servers` commented out exactly as stock does, so it is never offered any. It is pinned because its *presence* is what proves `configure` did not probe the build host — not because it runs. Its `service_condcommand ntpd restart` could not work here either; see the `91-ntp-kick` row. Upstream-worthy: Buildroot's `dhcpcd.mk` should pass `--with-hooks` itself. | | `/etc/resync` | **identical** | Byte-for-byte match (53 bytes: `( while [ 1 ]; do sync; sleep 5; done ) &`). Executable bit set. | | `/etc/proftpd.conf` | **identical, with a flagged security caveat** | Byte-for-byte reproduction of stock: `User root`/`Group root`, `RootLogin on`, anonymous ` AllowAll`, `Umask 000`. **Note, not acted on:** the package's own default `proftpd.conf` is meaningfully more hardened (`User nobody`, no root login, anonymous write denied). This project's task scope names exactly two sanctioned security improvements (resolv.conf's upstream default, ADR 0015's SSH host keys); silently hardening FTP as a third, undocumented one would change a well-known MiSTer workflow (anonymous/root FTP to `/media/fat`) without a maintainer decision or its own ADR. Shipped as stock parity; flagged here and in the task report as a candidate for a future ADR, not decided unilaterally. | | `/etc/samba/smb.conf` | **identical** | Byte-for-byte reproduction of stock (276 lines). `S91smb`'s stock double-guard (see above) means Samba cannot auto-start regardless — `/media/fat/linux/samba.sh` will never exist on a fresh image. | diff --git a/docs/wifi-parity.md b/docs/wifi-parity.md index ac584f1d..ceda78f5 100644 --- a/docs/wifi-parity.md +++ b/docs/wifi-parity.md @@ -98,7 +98,7 @@ path (stock's own README-level instructions) both depend on. | Contract element | Stock (`work/imgroot`) | Ours | Status | |---|---|---|---| -| `/etc/network/interfaces` | `wlan0`/`wlan1` `iface … inet manual` with `pre-up wpa_supplicant -s -B -P /run/wpa_supplicant.$IFACE.pid -i $IFACE -D nl80211,wext -c /media/fat/linux/wpa_supplicant.conf`, `post_up sleep 2`, `post-down killall -q wpa_supplicant` | `board/mister/de10nano/rootfs-overlay/etc/network/interfaces` | **Adapted (v9).** Stock's content, plus a 9-line header comment and one `pre-up i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info …` wait loop per `wlan` stanza — `diff` against `work/imgroot/etc/network/interfaces` exits 1 with exactly those 11 added lines (re-verified this task). Every stock directive is reproduced unchanged; nothing is removed. Authored by P2.3 (then byte-identical), diverged by `4cf2fc7` (v9); `docs/init-parity.md:147` carries the same row. | +| `/etc/network/interfaces` | `wlan0`/`wlan1` `iface … inet manual` with `pre-up wpa_supplicant -s -B -P /run/wpa_supplicant.$IFACE.pid -i $IFACE -D nl80211,wext -c /media/fat/linux/wpa_supplicant.conf`, `post_up sleep 2`, `post-down killall -q wpa_supplicant` | `board/mister/de10nano/rootfs-overlay/etc/network/interfaces` | **Adapted (v9, revised).** Stock's content, plus a header comment and two `pre-up` lines per `wlan` stanza: a `[ -e /sys/class/net/$IFACE ]` device-presence guard and the `i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info …` wait loop behind it — `diff` against `work/imgroot/etc/network/interfaces` exits 1 with exactly 47 added lines (43 comment + 4 code) and zero removals. Every stock directive is reproduced unchanged; nothing is removed. Authored by P2.3 (then byte-identical), diverged by `4cf2fc7` (v9); the guard was added later to stop `S40network`'s `ifup -a` burning the full 20 s on an absent `wlan1` (§9). `docs/init-parity.md:147` carries the same row. | | `/etc/init.d/S40network` | `ifup -a` / `ifdown -a` (ifupdown-scripts package default) | Not overlaid — `BR2_PACKAGE_IFUPDOWN_SCRIPTS`'s own Kconfig default (`default y if BR2_ROOTFS_SKELETON_DEFAULT`, `work/buildroot/package/ifupdown-scripts/Config.in`) auto-selects it; our defconfig sets neither `BR2_PACKAGE_SYSTEMD_NETWORKD` nor `BR2_PACKAGE_NETIFRC` (the two symbols that would suppress it) and leaves `BR2_ROOTFS_SKELETON_DEFAULT` at Buildroot's own default (y) | **Identical**, confirmed byte-for-byte by P2.3 (`docs/init-parity.md:63`); re-confirmed the selecting conditions still hold in this defconfig. | | `/etc/init.d/S41dhcpcd` | starts `dhcpcd` globally (no `-i`) | Package default, not overlaid; `BR2_PACKAGE_DHCPCD=y` (defconfig line 807, P2.1) | **Functionally identical** (P2.3 finding, `docs/init-parity.md:64`) — only the PID-file path differs, an artifact of the newer dhcpcd release, not a decision point. | | `/etc/dhcpcd.conf` | `hostname`, `clientid`, `option rapid_commit`, etc. enabled | `board/mister/de10nano/rootfs-overlay/etc/dhcpcd.conf` | **Identical.** `diff` exit 0 (re-verified this task). Authored by P2.3. | @@ -117,8 +117,8 @@ re-confirmed). correct, and every file P2.3 wrote still reproduces stock's directives. Verified, not assumed — `diff` was re-run against `work/imgroot` in this task: `/etc/dhcpcd.conf` is byte-identical (exit 0); `/etc/network/interfaces` -is stock plus v9's additive `pre-up` wait loop and its header comment (exit 1, -11 added lines, nothing removed — see the row above). +is stock plus v9's additive `pre-up` lines and their header comment (exit 1, +47 added lines, nothing removed — see the row above). ## 2. The `wifi.sh` contract itself @@ -896,6 +896,20 @@ also run `ifup wlan1` if `wlan1` is configured but has no device present — single-dongle hotplug. Naming the interface that actually fired (`%k`, which — per Divergence 1 — is also its permanent name) avoids that. +**The boot path had the identical bug, and needed its own fix.** `%k` only +narrows the *hotplug* caller; `S40network` still runs a literal `ifup -a`, so on +every boot of a single-dongle box the `wlan1` stanza polled its full 20 s for a +device that will never appear — with `S41dhcpcd`, `S49ntp`, `S50sshd` and +Main_MiSTer all serialised behind it in `rcS`. Measured, not estimated: running +the stanza's exact loop against an absent interface takes 20 s. The fix is a +`pre-up [ -e /sys/class/net/$IFACE ]` guard ahead of the loop in each stanza +(§1 table). It is safe precisely *because* this rule exists: a dongle that is +merely slow rather than absent still gets its `add` uevent, and this rule fires +on udev's boot-time coldplug (`S10udevd`'s `udevadm trigger`) as well as on +later insertion, so bring-up for a late device is event-driven rather than +polled. The v9 loop stays behind the guard as a cheap safety net for the narrow +case it was written for — netdev present, `nl80211` not yet ready. + **Why the detach matters is a boot-time regression avoided, not hygiene.** `udevadm settle --timeout=30` (`S10udevd:43`, above) waits on precisely the event this rule creates. A non-detached `RUN+=` would hold that event open diff --git a/scripts/check-linux-img.sh b/scripts/check-linux-img.sh index 4927b0f9..cc78bb7b 100755 --- a/scripts/check-linux-img.sh +++ b/scripts/check-linux-img.sh @@ -170,8 +170,12 @@ else # turns a future regression -- a Buildroot bump that changes the option, a host # that grows a daemon -- from a silent image difference into a red build. # Expected set = the canonical CI-built image: dhcpcd's three unconditional - # hooks, 50-ntp.conf (we ship ntpd), and our own 90-timezone (ADR 0025). - EXPECT_DHCPCD_HOOKS="01-test 20-resolv.conf 30-hostname 50-ntp.conf 90-timezone" + # hooks, 50-ntp.conf (we ship ntpd), and our own 90-timezone (ADR 0025) and + # 91-ntp-kick. NB 50-ntp.conf is dhcpcd's own and is inert here by design -- + # etc/dhcpcd.conf leaves `option ntp_servers` commented out (stock parity), so + # it is never offered any servers to merge. It is pinned because its PRESENCE + # is what proves configure did not probe the build host, not because it runs. + EXPECT_DHCPCD_HOOKS="01-test 20-resolv.conf 30-hostname 50-ntp.conf 90-timezone 91-ntp-kick" hooks_dir=$(find "$dump_dir" -type d -path '*/dhcpcd/dhcpcd-hooks' 2>/dev/null | head -1) if [ -z "$hooks_dir" ]; then bad "no dhcpcd-hooks directory in the image (dhcpcd or its hooks are missing)" diff --git a/scripts/ci-tests.sh b/scripts/ci-tests.sh index 761b8e18..77145518 100755 --- a/scripts/ci-tests.sh +++ b/scripts/ci-tests.sh @@ -1766,6 +1766,57 @@ else fi fi +# --- cold-boot time sync (91-ntp-kick) --------------------------------------- +# Same shape as the timezone hook above, and for the same reason: ntpd is started +# by S49ntp before any network exists, so on a no-RTC board the clock waits on +# ntpd's DNS retry backoff (2-3-4-6-8-12-16-24-32-48-64s) rather than on the +# network. This hook restarts ntpd once, when an address actually arrives. Assert +# the hook AND dhcpcd's runner, since a hook nothing sources is inert. +require_present "usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick" "dhcpcd 91-ntp-kick hook" +require_present "etc/init.d/S49ntp" "S49ntp (the hook restarts it; without it the kick is a no-op)" + +# `iburst` is what makes the kick worth making: without it ntpd falls back to +# minpoll and takes minutes to select a source, so the restart would buy little. +# It ships in the ntp package's own ntp.conf, which means a package bump could +# drop it with a green build. +if tar_has "etc/ntp.conf"; then + if tar xOf "$ROOTFS_TAR" ./etc/ntp.conf 2>/dev/null | grep -qE '^server .*[[:space:]]iburst'; then + pass "/etc/ntp.conf keeps iburst on its server lines" + else + fail "/etc/ntp.conf keeps iburst on its server lines" \ + "without iburst the 91-ntp-kick restart converges in minutes, not seconds" + fi +else + fail "/etc/ntp.conf present" "not in rootfs.tar" +fi + +# The behaviour -- once per boot and not once per event, never starting an ntpd +# that is not running, not spending its one kick on a pass that bailed out, and +# the two SOURCED properties -- has its own sandboxed harness. No build, no +# board, no network. +printf -- '--- test-ntp-kick.sh: ntpd kick hook behaviour (29 cases) ---\n' +if "$ROOT/scripts/test-ntp-kick.sh"; then + pass "test-ntp-kick.sh (ntpd kick hook behaviour, 29 cases)" +else + fail "test-ntp-kick.sh (ntpd kick hook behaviour, 29 cases)" \ + "one or more cases failed -- see output above" +fi + +if [ -z "$QEMU_ARM" ]; then + skip "test-ntp-kick.sh under the target's own BusyBox ash" "qemu-arm not found on PATH" +elif [ ! -x "$TARGET/bin/busybox" ]; then + skip "test-ntp-kick.sh under the target's own BusyBox ash" "$TARGET/bin/busybox not present" +else + printf -- '--- test-ntp-kick.sh: same cases, target BusyBox ash under qemu-arm ---\n' + if NTP_TEST_SH="$QEMU_ARM -L $TARGET $TARGET/bin/busybox sh" \ + "$ROOT/scripts/test-ntp-kick.sh"; then + pass "test-ntp-kick.sh under the target's own BusyBox ash" + else + fail "test-ntp-kick.sh under the target's own BusyBox ash" \ + "passes on the host shell but not on BusyBox ash -- see output above" + fi +fi + # ============================================================================= section "P3.10 — Network filesystem client parity (NFS half per ADR 0022)" # ============================================================================= diff --git a/scripts/test-ntp-kick.sh b/scripts/test-ntp-kick.sh new file mode 100755 index 00000000..1dd42f70 --- /dev/null +++ b/scripts/test-ntp-kick.sh @@ -0,0 +1,265 @@ +#!/usr/bin/env bash +# +# scripts/test-ntp-kick.sh — sandboxed functional test of the ntpd kick dhcpcd +# hook +# (board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick). +# +# WHY THIS EXISTS. The hook's whole contract is about WHEN it does nothing, and +# none of that is visible in a single successful run: +# +# - it must fire at most ONCE per boot. dhcpcd re-fires RENEW every few hours +# for the life of the session, and restarting ntpd each time would discard +# its accumulated clock discipline. You only see that break on the second +# event. +# - it must NOT claim its once-per-boot stamp on a pass that bailed out. The +# ordering case this was written around -- a wired box, where the hook fires +# during S41dhcpcd, BEFORE S49ntp has started ntpd -- depends entirely on +# that, and it is exactly the sort of thing a reordered guard breaks +# silently. +# - it must never start an ntpd that is not already running, so a by-hand or +# /etc/default/ntpd disable stands. +# - it is SOURCED into dhcpcd's shell, so "leaks nothing" and "never exits its +# caller" are correctness properties, not style. An `exit` here would end +# dhcpcd's whole hook run and take 20-resolv.conf and 30-hostname with it. +# +# ci-tests.sh's rootfs.tar checks can only see that the file shipped. This tests +# what it does. +# +# HOW. The hook is copied into a throwaway sandbox with its three absolute paths +# (/etc/init.d/S49ntp, /var/run/ntpd.pid, /run/ntp-kick) rewritten to point +# inside it, and the init script is stubbed with one that records every +# invocation. Liveness is real: the "running" cases use this harness's own PID, +# and the "dead" case uses a PID we reaped ourselves. Nothing here needs a build, +# a board, or a network. +# +# Each case sources the hook exactly as dhcpcd does, with $reason and $if_up in +# the environment. One sandbox copy has its `) &` rewritten to `)` so the body +# runs in the FOREGROUND and every assertion is deterministic instead of racing a +# background job; the unmodified copy is kept so the backgrounding itself is +# still asserted. +# +# Usage: scripts/test-ntp-kick.sh +# Exit 0 iff every case passed. Wired into scripts/ci-tests.sh, which runs it +# twice: once under the host shell, and once under the target's own BusyBox ash +# via qemu-arm -- the shell that will actually run this on the box. The host's +# /bin/sh is usually dash, a good POSIX proxy but not the same interpreter. +# +# Env: +# NTP_TEST_SH shell to run the script under (default: sh). May be a command +# with arguments, e.g. +# NTP_TEST_SH="qemu-arm -L output/target output/target/bin/busybox sh" + +# shellcheck disable=SC2030,SC2031 +# Subshell-scoped environment is the mechanism here, not an accident: each case +# sources the hook inside ( ) with its own $reason/$if_up, as dhcpcd-run-hooks +# does, so nothing carries between cases. + +set -u +# Deliberately not -e: run every case, report the whole picture -- same +# rationale as ci-tests.sh, test-initramfs.sh and test-timezone.sh. + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="$(cd "$HERE/.." && pwd)" +SRC="$ROOT/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick" + +if [ ! -f "$SRC" ]; then + echo "test-ntp-kick.sh: ERROR: $SRC not found" >&2 + exit 2 +fi + +read -r -a TEST_SH <<< "${NTP_TEST_SH:-sh}" + +SB="$(mktemp -d "${TMPDIR:-/tmp}/ntp-kick-test.XXXXXX")" +trap 'rm -rf "$SB"' EXIT + +NTP_INIT="$SB/S49ntp" +NTPD_PID="$SB/ntpd.pid" +STAMPDIR="$SB/ntp-kick" +CALLS="$SB/init.calls" + +# The stub stands in for /etc/init.d/S49ntp: record the verb, say nothing. +cat > "$NTP_INIT" <> "$CALLS" +EOF +chmod +x "$NTP_INIT" + +rewrite() { + sed -e "s|/etc/init.d/S49ntp|$NTP_INIT|g" \ + -e "s|/var/run/ntpd.pid|$NTPD_PID|g" \ + -e "s|/run/ntp-kick|$STAMPDIR|g" \ + "$SRC" +} + +# Two sandbox copies: +# sync — `) &` rewritten to `)`, so assertions are race-free +# async — untouched, so the backgrounding itself stays asserted +rewrite > "$SB/async" +rewrite | sed -E 's|^([[:space:]]*)\) &.*NTP-KICK-BACKGROUND.*|\1)|' > "$SB/sync" +if grep -qE '^[[:space:]]*\) &' "$SB/sync"; then + echo "test-ntp-kick.sh: ERROR: the sandbox copy is still asynchronous" >&2 + echo " (did the NTP-KICK-BACKGROUND marker move or change?)" >&2 + exit 2 +fi +if ! grep -qE '^[[:space:]]*\)$' "$SB/sync"; then + echo "test-ntp-kick.sh: ERROR: no subshell close found in the sandbox copy" >&2 + exit 2 +fi +if ! grep -qE '^[[:space:]]*\) &' "$SB/async"; then + echo "test-ntp-kick.sh: ERROR: the hook no longer backgrounds its body" >&2 + exit 2 +fi + +pass=0; fail=0; skipped=0 +ok() { printf ' PASS %s\n' "$1"; pass=$((pass + 1)); } +skip() { printf ' SKIP %s -- %s\n' "$1" "$2"; skipped=$((skipped + 1)); } +bad() { + printf ' FAIL %s\n' "$1" + [ -s "$SB/out" ] && sed 's/^/ console: /' "$SB/out" + fail=$((fail + 1)) +} +must() { local d="$1"; shift; if "$@"; then ok "$d"; else bad "$d"; fi; } +mustnt() { local d="$1"; shift; if "$@"; then bad "$d"; else ok "$d"; fi; } + +# A PID that is certainly gone: spawn, reap, then confirm. PID reuse would make +# this a false negative rather than a false pass, and it is checked, not assumed. +DEADPID="$( ( : ) & echo $! )" +wait 2>/dev/null + +# reboot — a fresh boot: tmpfs stamp gone, no record of past calls. +reboot_sim() { rm -rf "$STAMPDIR"; rm -f "$CALLS"; } + +# ntpd_running / ntpd_stopped / ntpd_stale — the three pidfile states. +ntpd_running() { printf '%s\n' "$$" > "$NTPD_PID"; } +ntpd_stopped() { rm -f "$NTPD_PID"; } +ntpd_stale() { printf '%s\n' "$DEADPID" > "$NTPD_PID"; } + +# fire [reason] [if_up] -- one dhcpcd address event, sourced the way +# dhcpcd-run-hooks sources it, INSIDE ${TEST_SH[@]}. Running it under the +# harness's own shell would make NTP_TEST_SH decorative and the "target BusyBox +# ash" CI leg a no-op that prints PASS. +# +# Pass the literal string "unset" as if_up to leave it undefined -- a state +# dhcpcd never produces but a sourced file must survive. An EMPTY if_up is a +# distinct case, hence ${2-true} rather than ${2:-true}. +# +# The inner shell touches its second argument after sourcing: reached only if +# the hook did not exit its caller. +fire() { + # shellcheck disable=SC2016 # $1/$2 belong to the inner shell, on purpose + _src='. "$1"; : > "$2"' + if [ "${2-}" = unset ]; then + env -u if_up reason="${1:-BOUND}" \ + "${TEST_SH[@]}" -c "$_src" _ \ + "$SB/sync" "$SB/returned-to-dhcpcd" > "$SB/out" 2>&1 + else + env reason="${1:-BOUND}" if_up="${2-true}" \ + "${TEST_SH[@]}" -c "$_src" _ \ + "$SB/sync" "$SB/returned-to-dhcpcd" > "$SB/out" 2>&1 + fi +} + +# Invoked indirectly, as `must`/`mustnt` arguments. +# shellcheck disable=SC2329 +kicked() { [ -s "$CALLS" ]; } +# shellcheck disable=SC2329 +kicks() { [ -f "$CALLS" ] && [ "$(wc -l < "$CALLS")" -eq "$1" ]; } +# shellcheck disable=SC2329 +stamped() { [ -d "$STAMPDIR" ]; } + +printf '\n--- the kick itself ---\n' +reboot_sim; ntpd_running +fire BOUND true +must "BOUND with ntpd running kicks it" kicked +must "the verb is restart" grep -qx 'restart' "$CALLS" +must "the once-per-boot stamp is claimed" stamped + +printf '\n--- once per boot, not once per event ---\n' +fire RENEW true +must "a later RENEW does not kick again" kicks 1 +fire BOUND true +must "a later BOUND does not kick again" kicks 1 +reboot_sim; ntpd_running +fire BOUND true +must "the stamp re-arms on the next boot (tmpfs)" kicks 1 + +printf '\n--- ntpd that is not running is left alone ---\n' +reboot_sim; ntpd_stopped +fire BOUND true +mustnt "no pidfile: does not start ntpd" kicked +mustnt "no pidfile: the stamp is NOT spent" stamped +reboot_sim; ntpd_stale +fire BOUND true +mustnt "stale pidfile (dead pid): does not kick" kicked +mustnt "stale pidfile: the stamp is NOT spent" stamped +reboot_sim; printf 'not-a-pid\n' > "$NTPD_PID" +fire BOUND true +mustnt "garbage pidfile: does not kick" kicked +mustnt "garbage pidfile: the stamp is NOT spent" stamped +reboot_sim; : > "$NTPD_PID" +fire BOUND true +mustnt "empty pidfile: does not kick" kicked + +# The ordering case the hook was written around: on a wired box this fires +# during S41dhcpcd, before S49ntp has started ntpd. The pass must bail out +# WITHOUT spending the stamp, so a genuinely late interface still gets its kick. +printf '\n--- the wired-box ordering case ---\n' +reboot_sim; ntpd_stopped +fire BOUND true +mustnt "fires before ntpd exists: no kick" kicked +ntpd_running +fire BOUND true +must "a later address event still gets its one kick" kicked + +printf '\n--- events that must do nothing ---\n' +reboot_sim; ntpd_running +fire BOUND false +mustnt "if_up=false does not kick" kicked +fire BOUND '' +mustnt "empty if_up does not kick" kicked +fire BOUND unset +mustnt "unset if_up does not kick" kicked +fire NOCARRIER true +mustnt "NOCARRIER does not kick" kicked +fire PREINIT true +mustnt "PREINIT does not kick" kicked +fire EXPIRE true +mustnt "EXPIRE does not kick" kicked +mustnt "none of the above spent the stamp" stamped + +printf '\n--- missing init script ---\n' +reboot_sim; ntpd_running; chmod -x "$NTP_INIT" +fire BOUND true +mustnt "non-executable S49ntp: does not kick" kicked +mustnt "non-executable S49ntp: the stamp is NOT spent" stamped +chmod +x "$NTP_INIT" + +printf '\n--- properties it has because it is SOURCED ---\n' +reboot_sim; ntpd_running +rm -f "$SB/returned-to-dhcpcd" +fire BOUND true +must "never exits its caller (20-resolv.conf still runs)" \ + test -f "$SB/returned-to-dhcpcd" +# Every variable the hook sets lives inside its subshell; dhcpcd's shell must +# come back clean. Checked in the shell under test, not this one. +# shellcheck disable=SC2016 # $v/$1 belong to the inner shell, on purpose +leak_check='. "$1"; for v in NTP_INIT NTPD_PID STAMPDIR ntpd_pid; do + eval "val=\${$v-UNSET}"; [ "$val" = UNSET ] || { echo "LEAKED $v"; exit 1; } +done; exit 0' +reboot_sim; ntpd_running +if env reason=BOUND if_up=true "${TEST_SH[@]}" -c "$leak_check" _ "$SB/sync" \ + > "$SB/out" 2>&1; then + ok "leaks no variables into dhcpcd's shell" +else + bad "leaks no variables into dhcpcd's shell" +fi + +printf '\n--- structural ---\n' +must "the shipped hook has no shebang (it is sourced)" \ + test "$(head -c 2 "$SRC")" != '#!' +mustnt "the shipped hook is not executable" test -x "$SRC" +must "the shipped hook still backgrounds its body" \ + grep -qE '^[[:space:]]*\) &' "$SB/async" + +printf '\n%d passed, %d failed, %d skipped\n' "$pass" "$fail" "$skipped" +[ "$fail" -eq 0 ] From 0644618539fde3ad914b7080a825612f86e0185c Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 3 Sep 2026 14:48:36 -0500 Subject: [PATCH 2/3] review: kick only on address acquisition, and count every iburst line Both findings from the Copilot review of #147. Both were real. 1. RENEW/REBIND could restart a long-synchronised ntpd. The reason set was copied from 90-timezone (BOUND|REBOOT|REBIND|RENEW + *6), which is right for a hook gated on a persistent file it writes once, and wrong here: this hook's guard is deliberately re-armable, so a pass that bails out leaves the kick available for any later matching event. Match a renewal and that "later event" includes one arriving hours into the session, against an ntpd that has been synchronised the whole time -- discarding its accumulated clock discipline for nothing. The wired ordering case makes it reachable rather than theoretical: it always bails out first (S41dhcpcd blocks until the first lease, so the hook fires before S49ntp has started ntpd), leaving the kick armed for exactly such an event. Narrowed to BOUND|REBOOT|BOUND6|REBOOT6. The review suggested keeping REBIND; dhcpcd's own selection logic says otherwise -- src/dhcp.c:2499-2513 picks RENEW *and* REBIND only when `state->old` is non-NULL, i.e. the address was already there, differing just in whether the address or server changed. Neither is an address ARRIVING. BOUND is a new lease and REBOOT is a cached lease confirmed at startup (DHS_REBOOT); those two are the acquisitions. Nothing is lost by narrowing: every case this hook was written for -- WiFi associating minutes into a boot, a dongle plugged in later, an AP that comes back after the box is up -- is a new lease, hence BOUND. And an address that changes under a RENEW/REBIND needs no kick anyway, since ntpd holds peers by then and the HAVE_RTNETLINK path reattaches them within ~3s. 2. The iburst assertion could pass with iburst on only some server lines. `grep -qE '^server .*iburst'` matched if ANY line carried it, while the message claimed all of them. A bump dropping iburst from two of the four pool lines would have gone green, leaving ntpd at minpoll for those -- the exact regression the check exists to catch. Now counts `^server` lines against those carrying iburst and requires equal and non-zero, reporting n/total either way. Verified against the real /etc/ntp.conf (4/4 pass), one line stripped (3/4 fail), all stripped (0/4 fail), no server lines at all (fails with its own message), and iburst followed by another option (pass). Tests: 29 -> 34 cases, all passing on the host shell and on the target's own BusyBox ash under qemu-arm. New cases cover REBOOT as an acquisition, the renewal-after-bailout scenario end to end, and RENEW/REBIND as no-ops with ntpd up and the stamp unspent. Mutation-checked: restoring RENEW|REBIND to the case list fails 5 of them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014SBpBTbtBsFwjAgKCx5T9S --- .../usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick | 26 ++++++++++- docs/init-parity.md | 2 +- scripts/ci-tests.sh | 23 +++++++--- scripts/test-ntp-kick.sh | 45 +++++++++++++------ 4 files changed, 73 insertions(+), 23 deletions(-) diff --git a/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick b/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick index 1f81a1d6..b0b753a6 100644 --- a/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick +++ b/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick @@ -94,12 +94,36 @@ # and take 20-resolv.conf and 30-hostname with it. The subshell answers both at # once, and keeps every variable below out of dhcpcd's shell. # +# ACQUISITION REASONS ONLY -- NOT RENEW OR REBIND. +# 90-timezone matches BOUND|REBOOT|REBIND|RENEW and their *6 forms, which is +# right for a hook gated on a persistent file it writes once. It is WRONG here, +# because this hook's guard is deliberately re-armable: a pass that bails out +# does not spend the stamp (see above), so any later matching event can still +# fire the kick. Match a renewal and that "later event" includes one arriving +# hours into the session, against an ntpd that has been synchronised the whole +# time -- restarting it there would discard its accumulated clock discipline for +# nothing. The wired ordering case makes that reachable rather than theoretical: +# it always bails out first, leaving the kick armed for exactly such an event. +# +# The distinction is in dhcpcd's own selection logic (src/dhcp.c:2499-2513): +# RENEW and REBIND are both chosen only when `state->old` is non-NULL -- we +# already had a lease, and merely extended it (RENEW) or moved it to another +# server or address (REBIND). Neither is an address ARRIVING, which is the event +# this hook exists to catch. BOUND is a new lease and REBOOT is a cached lease +# confirmed at startup (DHS_REBOOT); those two are the acquisitions. +# +# Nothing is lost by narrowing: every case this hook was written for -- WiFi +# associating minutes into a boot, a dongle plugged in later, an AP that comes +# back after the box is up -- is a NEW lease, hence BOUND. And an address that +# changes under a RENEW/REBIND needs no kick anyway: ntpd already holds peers by +# then, and the HAVE_RTNETLINK path above reattaches them within ~3s. +# # The *6 reasons cannot fire on this kernel -- it is built without IPv6 # (linux.config: "# CONFIG_IPV6 is not set") -- but they cost nothing and are # correct the day that changes. case "$reason" in -BOUND|REBOOT|REBIND|RENEW|BOUND6|REBOOT6|REBIND6|RENEW6) +BOUND|REBOOT|BOUND6|REBOOT6) # if_up is compared as DATA, not executed. dhcpcd's own hooks write # `if $if_up`, which works because dhcpcd sets it to the literal string # true or false -- but this is sourced into dhcpcd's shell, where diff --git a/docs/init-parity.md b/docs/init-parity.md index 24d4a4c0..8a1d0921 100644 --- a/docs/init-parity.md +++ b/docs/init-parity.md @@ -152,7 +152,7 @@ regardless, so the worst case is slow, never weak. | `/etc/inittab` | **adapted** (3 documented deviations) | Full stock shape reproduced (`::sysinit:/media/fat/MiSTer &`, `/etc/resync &`, `rcS`, shutdown sequence) with: **(1)** the remount-rw sysinit line kept **commented out**, exactly as stock has it — Buildroot's own skeleton default inittab ships this line **uncommented**, which would remount `/` rw at every sysinit and defeat the whole read-only-root design (ADR 0011); confirmed this project's own finalize hook tries to uncomment it too (see below) and is overridden by the overlay running last. **(2)** the serial console runs `ttyS0::respawn:/sbin/agetty --nohostname -L ttyS0 115200 vt100` — util-linux `agetty`, matching stock (whose inittab also uses `agetty --nohostname`), now that `BR2_PACKAGE_UTIL_LINUX_AGETTY` is enabled (see `docs/util-linux-parity.md`). It still targets `ttyS0` explicitly rather than stock's `console` alias, because this board's actual cmdline is `console=ttyS0,115200` (`docs/boot-chain.md`). agetty treats a numeric positional argument as the baud rate, so `ttyS0 115200` and `115200 ttyS0` are equivalent; the port-first order is kept for continuity with the previous BusyBox `getty` line. (Earlier revisions used BusyBox `getty` because `BR2_PACKAGE_UTIL_LINUX_BINARIES` was not selected — that constraint no longer holds; the overlapping BusyBox `getty` applet is now disabled so `agetty` is the console.) **(3)** `gpm` is invoked at `/usr/sbin/gpm`, not stock's `/sbin/gpm` — originally **a real bug caught during acceptance verification**: gpm is a real package (not a BusyBox applet, which lands under `/sbin` regardless of usr-merge), and when this row was first written (P2.3) the rootfs was **not** usr-merged — `/sbin` was a real directory, not `-> /usr/sbin` the way stock's `work/imgroot/sbin` is — so `/sbin/gpm` genuinely did not exist in the image. An inittab `sysinit` line is an **absolute path**, so unlike `/etc/profile`'s `PATH` it gets no shell search at all; it would have silently failed to spawn on every single boot. Caught by exhaustively grep'ing every absolute path referenced anywhere in the overlay against the actually-built image (see the task report) — first build had this wrong; fixed and rebuilt before acceptance. **Since P2.9 v2** (`7be9ee5`) set `BR2_ROOTFS_MERGED_USR=y`, that premise no longer holds: this build is now usr-merged like stock (verified: `output/target/sbin -> usr/sbin`), so `/sbin/gpm` *would* resolve today. **The deviation is kept anyway, deliberately.** Buildroot builds gpm with `--prefix=/usr`, so the binary's real path is `/usr/sbin/gpm` in *either* layout (gpm's own file list: `gpm,./usr/sbin/gpm`) — stock's binary is at that same physical path, and stock's inittab only reaches it through the `/sbin -> usr/sbin` compat symlink that exists solely because stock is usr-merged. So our path is correct in both layouts and stock's is correct in only one; ours survives an unmerge, stock's would break on one. Given P2.3 already got bitten by exactly that (the unmerged build had no `/sbin/gpm` at all), pointing at the real path rather than a merge-dependent alias is the more durable choice, even though a future unmerge is unlikely. This is an **accepted permanent deviation from stock**, not a cleanup waiting to happen. **Also dropped — later RESTORED, guarded (T3, 2026-07-27):** `loadkeys /etc/kbd.map` and `setfont` were dropped by P2.3 because this BusyBox build has no `loadkeys` applet at all and `CONFIG_SETFONT` is explicitly not set (still true — re-verified in `output/build/busybox-1.38.0/.config`; busybox's `loadkmap` applet is not a substitute, it reads binary bkeymap, not stock's text keymap); keeping either line verbatim would just fail every boot. T3 vendored stock's `etc/kbd.map` (it blanks the F12/Mute/Vol± keycodes Main_MiSTer consumes via evdev) and restored both lines wrapped in `[ -x /usr/bin/... ]` guards: with the parallel T5 task's `BR2_PACKAGE_KBD` (the same package stock's own loadkeys/setfont came from) the lines do stock's exact job, without it they are silent no-ops instead of boot errors. See the inittab's own note 3 and `docs/stock-reconciliation.md` §3c. `gpm -m /dev/input/mice -t imps2` itself **is** kept (just at the corrected path) — `BR2_PACKAGE_GPM=y` was deliberately selected in P2.1 for this, and the binary is present. | | `/etc/profile` | **adapted** (1 documented deviation) | Full stock content reproduced (PATH, `PS1='$(pwd)# '`, `EDITOR=/bin/vi`, `/etc/profile.d/*.sh` sourcing, `LC_ALL=en_US.UTF-8`, and critically the login-time `mount -o remount,rw /` — this is how `/` ever becomes writable at all, matching stock and ADR 0011's own description of the mechanism). **Deviation:** `PATH` gains an explicit `/bin:/sbin:` prefix stock's literal string doesn't have. Stock's rootfs is usr-merged (`work/imgroot`: `/bin -> usr/bin`, `/sbin -> usr/sbin`), so its `PATH="/usr/bin:/usr/sbin"` already covered `/bin`/`/sbin` for free. When this deviation was introduced (P2.3) *this* build was a plain (non-merged) skeleton layout, so omitting `/bin:/sbin` from `PATH` would have silently dropped most BusyBox applets from every interactive shell; changing `BR2_ROOTFS_MERGED_USR` was then out of P2.3's scope ("do NOT disturb ... the package set"), making the overlay `PATH` the correct-altitude fix. **Since P2.9 v2** (`7be9ee5`) set `BR2_ROOTFS_MERGED_USR=y` — the change that fixed the `/lib/security/pam_unix.so` SSH lockout — this build is usr-merged too, so the `/bin:/sbin:` prefix is now redundant. It is kept because it is harmless (the paths resolve to the same directories) and matches Buildroot's own skeleton default profile. The file's own header comment records the same history. | | `/usr/lib/dhcpcd/dhcpcd-hooks/90-timezone` | **added (divergence, [ADR 0025](decisions/0025-first-boot-timezone-autodetect.md))** | Stock has no equivalent, and that is the gap it closes: `/etc/localtime` points at `/media/fat/linux/timezone`, which **does not exist on a fresh card**, so glibc falls back to UTC silently and permanently. The first time dhcpcd brings an interface up with an address, this asks `ip-api.com` for the zone of the box's public IP and copies `/usr/share/zoneinfo/posix/` to that path — the *same* provider, destination and file format as the community `Scripts_MiSTer/timezone.sh` "Automatic" mode, so the two are interchangeable. No new package (`curl` and tzdata were already in the image). **Note this adds no init script**: an earlier revision had an `S48timezone` as well, but on a DHCP box the lease has usually not landed by S48, so it was near-redundant with this hook — and it carried a `/proc/net/route` check whose IPv6 arm silently matched the kernel's own `ip6_null_entry`. A static-IP box configured only in `/etc/network/interfaces` never runs dhcpcd and so never autodetects; accepted deliberately, since setting a static address is already a by-hand act. Properties worth stating because they are the design: the guess is spent **once, and only when it was actually made** (gated on the timezone file *and* on a `timezone.autodetect` stamp written only when a provider answered *with a zone name*, so neither being offline nor a captive portal's HTTP 200 burns it), it **never overwrites** a timezone anyone has already set (re-checked immediately before the write, not only at the gate), and it **delays nothing** — the body is a backgrounded subshell, which also keeps it from leaking a single variable or function into dhcpcd's shell. Sourced, not executed (`dhcpcd-run-hooks`: `. "$hook"`), hence no shebang, no exec bit, and no `exit` — an `exit` here would end dhcpcd's whole hook run and take `20-resolv.conf`/`30-hostname` with it. Zone names arrive off the network, so they are validated against the shipped zoneinfo before being used as a path; `scripts/test-timezone.sh` asserts each rejection, and mutation-checks the two that could otherwise pass vacuously. | -| `/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick` | **added (divergence)** | Stock has no equivalent. The gap: this board has no RTC, so a cold boot starts at the epoch, and `S49ntp` launches `ntpd -g` at a fixed point in `rcS` whether or not a network exists — which on the common path it does not, since WiFi association plus DHCP normally completes well after S49. ntpd survives that in one half and not the other. The **interface** half heals itself: ntpd is built with `HAVE_RTNETLINK` (`config.h:786`), so an address appearing is a netlink event and `ntp_io.c:1980` → `ntp_peer.c:763` `refresh_all_peerinterfaces` reattaches peers within ~3 s. The **DNS** half does not: for a `server ` line the peer is created *only* in the DNS callback (`ntp_config.c:4459-4463` → `peer_name_resolved`), and at S49 there is no `/etc/resolv.conf` — `20-resolv.conf` writes it in this very hook pass — so resolution fails and **no peers exist at all** for the rescan to reattach. The clock then waits on ntpd's DNS retry backoff and nothing else: `libntp/ntp_intres.c` `manage_dns_retry_interval` walks 2-3-4-6-8-12-16-24-32-48-64 s (`config.h:1313` leaves `IGNORE_DNS_ERRORS` undefined, so `DNSFLAGS` is 0 and `retmax` is 64, not 1024), putting attempts at t = 2, 5, 9, 15, 23, 35, 51, 75, 107, 155, 219, 283 … s after ntpd started. Measured cost of the next-slot wait: a few seconds on a wired box, 10-20 s on a typical WiFi boot, 30 s to a couple of minutes when the network turns up late. This restarts ntpd once, when an address actually arrives, so it re-resolves against the `resolv.conf` that now exists; `/etc/ntp.conf`'s `iburst` and `S49ntp`'s `-g` then give correct time within ~10-15 s. **Not `sntp`**: `BR2_PACKAGE_NTP_SNTP` would also install Buildroot's `S48sntp` (`package/ntp/ntp.mk:110-114`), which runs sntp at boot one script *before* ntpd with no network, and would have to be suppressed — not worth ~10 s. **Not `service_condcommand`** (which `50-ntp.conf` uses): dhcpcd's `detect_init` finds no systemctl/rc-service/invoke-rc.d/service/sv here and falls through to its `/etc/init.d` branch, which tests `[ -x /etc/init.d/ntpd ]` — ours is `S49ntp` — and `service_status` then runs `$x/$1 status`, a verb `S49ntp` has no case for; both fail silently. The hook calls the script directly. **Not a udev rule**: a `net` `add` uevent fires when the netdev is *created*, before association, lease, route or `resolv.conf`, which would reproduce the bug one layer down. Design properties: fires **at most once per boot** (`mkdir /run/ntp-kick` is the atomic test-and-set, on a tmpfs, so it re-arms each boot — `RENEW` recurs for the life of the session and restarting ntpd each time would discard its accumulated clock discipline); **never starts an ntpd that is not running**, so a by-hand or `/etc/default/ntpd` disable stands; and the stamp is claimed **last**, after every gate, so a pass that bails out leaves the one kick available — which is what makes the wired-box ordering case correct, where `S41dhcpcd` blocks until the first lease and so fires this hook *before* `S49ntp` has started ntpd at all. Backgrounded subshell (`S49ntp restart` contains a `sleep 1`), sourced not executed — no shebang, no exec bit, no `exit`. A static-IP box never runs dhcpcd and never gets kicked; accepted, the same gap ADR 0025 accepts, and ntpd's own backoff still converges there. `scripts/test-ntp-kick.sh` asserts the behaviour in 29 sandboxed cases (run twice by `ci-tests.sh`: host shell, and the target's own BusyBox ash under qemu-arm), mutation-checked against dropping the once-per-boot gate and the liveness check. | +| `/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick` | **added (divergence)** | Stock has no equivalent. The gap: this board has no RTC, so a cold boot starts at the epoch, and `S49ntp` launches `ntpd -g` at a fixed point in `rcS` whether or not a network exists — which on the common path it does not, since WiFi association plus DHCP normally completes well after S49. ntpd survives that in one half and not the other. The **interface** half heals itself: ntpd is built with `HAVE_RTNETLINK` (`config.h:786`), so an address appearing is a netlink event and `ntp_io.c:1980` → `ntp_peer.c:763` `refresh_all_peerinterfaces` reattaches peers within ~3 s. The **DNS** half does not: for a `server ` line the peer is created *only* in the DNS callback (`ntp_config.c:4459-4463` → `peer_name_resolved`), and at S49 there is no `/etc/resolv.conf` — `20-resolv.conf` writes it in this very hook pass — so resolution fails and **no peers exist at all** for the rescan to reattach. The clock then waits on ntpd's DNS retry backoff and nothing else: `libntp/ntp_intres.c` `manage_dns_retry_interval` walks 2-3-4-6-8-12-16-24-32-48-64 s (`config.h:1313` leaves `IGNORE_DNS_ERRORS` undefined, so `DNSFLAGS` is 0 and `retmax` is 64, not 1024), putting attempts at t = 2, 5, 9, 15, 23, 35, 51, 75, 107, 155, 219, 283 … s after ntpd started. Measured cost of the next-slot wait: a few seconds on a wired box, 10-20 s on a typical WiFi boot, 30 s to a couple of minutes when the network turns up late. This restarts ntpd once, when an address actually arrives, so it re-resolves against the `resolv.conf` that now exists; `/etc/ntp.conf`'s `iburst` and `S49ntp`'s `-g` then give correct time within ~10-15 s. **Not `sntp`**: `BR2_PACKAGE_NTP_SNTP` would also install Buildroot's `S48sntp` (`package/ntp/ntp.mk:110-114`), which runs sntp at boot one script *before* ntpd with no network, and would have to be suppressed — not worth ~10 s. **Not `service_condcommand`** (which `50-ntp.conf` uses): dhcpcd's `detect_init` finds no systemctl/rc-service/invoke-rc.d/service/sv here and falls through to its `/etc/init.d` branch, which tests `[ -x /etc/init.d/ntpd ]` — ours is `S49ntp` — and `service_status` then runs `$x/$1 status`, a verb `S49ntp` has no case for; both fail silently. The hook calls the script directly. **Not a udev rule**: a `net` `add` uevent fires when the netdev is *created*, before association, lease, route or `resolv.conf`, which would reproduce the bug one layer down. Design properties: fires **at most once per boot** (`mkdir /run/ntp-kick` is the atomic test-and-set, on a tmpfs, so it re-arms each boot — two interfaces can `BOUND` at the same moment, and restarting ntpd more than once would discard its accumulated clock discipline for nothing); **never starts an ntpd that is not running**, so a by-hand or `/etc/default/ntpd` disable stands; and the stamp is claimed **last**, after every gate, so a pass that bails out leaves the one kick available — which is what makes the wired-box ordering case correct, where `S41dhcpcd` blocks until the first lease and so fires this hook *before* `S49ntp` has started ntpd at all. Backgrounded subshell (`S49ntp restart` contains a `sleep 1`), sourced not executed — no shebang, no exec bit, no `exit`. A static-IP box never runs dhcpcd and never gets kicked; accepted, the same gap ADR 0025 accepts, and ntpd's own backoff still converges there. **Acquisition reasons only** — `BOUND`/`REBOOT` and their `*6` forms, deliberately *not* `RENEW`/`REBIND` as `90-timezone` matches: dhcpcd picks those two only when `state->old` is non-NULL (`src/dhcp.c:2499-2513`), i.e. the address was already there. Matching them combines badly with the re-armable stamp — a renewal arriving hours into the session would restart an ntpd that had been synchronised the whole time, and the wired ordering case makes that reachable rather than theoretical, since it always bails out first and leaves the kick armed. Nothing is lost: WiFi associating late, a dongle plugged in later and an AP returning are all *new* leases, hence `BOUND`; and an address that merely changes under a `RENEW`/`REBIND` needs no kick, because ntpd holds peers by then and the `HAVE_RTNETLINK` path reattaches them in ~3 s. (Found in review of PR #147.) `scripts/test-ntp-kick.sh` asserts the behaviour in 34 sandboxed cases (run twice by `ci-tests.sh`: host shell, and the target's own BusyBox ash under qemu-arm), mutation-checked against dropping the once-per-boot gate, the liveness check, and the narrowed reason set. | | `/usr/lib/dhcpcd/dhcpcd-hooks/` (the set) | **pinned (build-host independence)** | dhcpcd's `configure` chooses which hooks to install by probing the **build host** for `ntpd`/`chronyd`/`systemd-timesyncd`/`ypbind`, and Buildroot's `dhcpcd.mk` passes no `--with-hooks`, so the image inherited whatever daemons the build machine happened to have: a GitHub runner produced `50-ntp.conf`, a developer box with timesyncd produced `50-timesyncd.conf` — with a green build both times (found 2026-09-02 by diffing a CI image against a local one). `external.mk` now appends `--with-hooks=ntp.conf --with-eghooks=yp.conf` to `DHCPCD_CONFIG_OPTS`, which reproduces the canonical CI image exactly (`01-test`, `20-resolv.conf`, `30-hostname`, `50-ntp.conf`, plus our `90-timezone` and `91-ntp-kick`; `50-yp.conf` stays an example under `/usr/share/dhcpcd/hooks`), and `scripts/check-linux-img.sh` asserts that exact set. `50-ntp.conf` is the right hook to pin given we ship classic `ntpd` (stock parity), though it is **inert in this image**: it merges DHCP-offered servers into `/etc/ntp.conf`, and `etc/dhcpcd.conf` leaves `option ntp_servers` commented out exactly as stock does, so it is never offered any. It is pinned because its *presence* is what proves `configure` did not probe the build host — not because it runs. Its `service_condcommand ntpd restart` could not work here either; see the `91-ntp-kick` row. Upstream-worthy: Buildroot's `dhcpcd.mk` should pass `--with-hooks` itself. | | `/etc/resync` | **identical** | Byte-for-byte match (53 bytes: `( while [ 1 ]; do sync; sleep 5; done ) &`). Executable bit set. | | `/etc/proftpd.conf` | **identical, with a flagged security caveat** | Byte-for-byte reproduction of stock: `User root`/`Group root`, `RootLogin on`, anonymous ` AllowAll`, `Umask 000`. **Note, not acted on:** the package's own default `proftpd.conf` is meaningfully more hardened (`User nobody`, no root login, anonymous write denied). This project's task scope names exactly two sanctioned security improvements (resolv.conf's upstream default, ADR 0015's SSH host keys); silently hardening FTP as a third, undocumented one would change a well-known MiSTer workflow (anonymous/root FTP to `/media/fat`) without a maintainer decision or its own ADR. Shipped as stock parity; flagged here and in the task report as a candidate for a future ADR, not decided unilaterally. | diff --git a/scripts/ci-tests.sh b/scripts/ci-tests.sh index 77145518..6b932fce 100755 --- a/scripts/ci-tests.sh +++ b/scripts/ci-tests.sh @@ -1779,12 +1779,21 @@ require_present "etc/init.d/S49ntp" "S49ntp (the hook restarts it; without it th # minpoll and takes minutes to select a source, so the restart would buy little. # It ships in the ntp package's own ntp.conf, which means a package bump could # drop it with a green build. +# EVERY server line, not merely one of them: a bump that drops iburst from some +# of the pool lines would leave ntpd waiting at minpoll for those, which is the +# regression this is here to catch. Counted rather than grep -q'd. if tar_has "etc/ntp.conf"; then - if tar xOf "$ROOTFS_TAR" ./etc/ntp.conf 2>/dev/null | grep -qE '^server .*[[:space:]]iburst'; then - pass "/etc/ntp.conf keeps iburst on its server lines" + ntp_conf_body=$(tar xOf "$ROOTFS_TAR" ./etc/ntp.conf 2>/dev/null) + n_server=$(printf '%s\n' "$ntp_conf_body" | grep -cE '^server[[:space:]]') + n_iburst=$(printf '%s\n' "$ntp_conf_body" | grep -cE '^server[[:space:]].*[[:space:]]iburst([[:space:]]|$)') + if [ "$n_server" -eq 0 ]; then + fail "/etc/ntp.conf keeps iburst on every server line" \ + "no 'server' lines at all -- 91-ntp-kick would have nothing to resolve" + elif [ "$n_server" -eq "$n_iburst" ]; then + pass "/etc/ntp.conf keeps iburst on every server line ($n_iburst/$n_server)" else - fail "/etc/ntp.conf keeps iburst on its server lines" \ - "without iburst the 91-ntp-kick restart converges in minutes, not seconds" + fail "/etc/ntp.conf keeps iburst on every server line" \ + "only $n_iburst of $n_server server lines carry it; without iburst the 91-ntp-kick restart converges in minutes, not seconds" fi else fail "/etc/ntp.conf present" "not in rootfs.tar" @@ -1794,11 +1803,11 @@ fi # that is not running, not spending its one kick on a pass that bailed out, and # the two SOURCED properties -- has its own sandboxed harness. No build, no # board, no network. -printf -- '--- test-ntp-kick.sh: ntpd kick hook behaviour (29 cases) ---\n' +printf -- '--- test-ntp-kick.sh: ntpd kick hook behaviour (34 cases) ---\n' if "$ROOT/scripts/test-ntp-kick.sh"; then - pass "test-ntp-kick.sh (ntpd kick hook behaviour, 29 cases)" + pass "test-ntp-kick.sh (ntpd kick hook behaviour, 34 cases)" else - fail "test-ntp-kick.sh (ntpd kick hook behaviour, 29 cases)" \ + fail "test-ntp-kick.sh (ntpd kick hook behaviour, 34 cases)" \ "one or more cases failed -- see output above" fi diff --git a/scripts/test-ntp-kick.sh b/scripts/test-ntp-kick.sh index 1dd42f70..df17e6d1 100755 --- a/scripts/test-ntp-kick.sh +++ b/scripts/test-ntp-kick.sh @@ -174,15 +174,39 @@ must "BOUND with ntpd running kicks it" kicked must "the verb is restart" grep -qx 'restart' "$CALLS" must "the once-per-boot stamp is claimed" stamped +reboot_sim; ntpd_running +fire REBOOT true +must "REBOOT (a cached lease confirmed at startup) kicks too" kicked + printf '\n--- once per boot, not once per event ---\n' -fire RENEW true -must "a later RENEW does not kick again" kicks 1 +reboot_sim; ntpd_running fire BOUND true -must "a later BOUND does not kick again" kicks 1 +fire BOUND true +must "a second BOUND does not kick again" kicks 1 reboot_sim; ntpd_running fire BOUND true must "the stamp re-arms on the next boot (tmpfs)" kicks 1 +printf '\n--- a renewal is not an acquisition ---\n' +# dhcpcd picks RENEW/REBIND only when state->old is non-NULL (src/dhcp.c: +# 2499-2513) -- the address was already there. Matching them would combine badly +# with the deliberately re-armable stamp: a renewal arriving hours into the +# session would restart an ntpd that has been synchronised the whole time, +# discarding its accumulated discipline. The wired ordering case is what makes +# that reachable rather than theoretical, since it always bails out first and +# leaves the kick armed. Found in review of PR #147. +reboot_sim; ntpd_stopped +fire BOUND true # fires during S41dhcpcd, before S49ntp +mustnt "wired ordering bails out" kicked +mustnt "...leaving the kick armed" stamped +ntpd_running # hours later; ntpd long since synchronised +fire RENEW true +mustnt "a RENEW hours later does not restart a synced ntpd" kicked +fire REBIND true +mustnt "a REBIND hours later does not either" kicked +fire BOUND true +must "...but a genuine new lease still gets its kick" kicked + printf '\n--- ntpd that is not running is left alone ---\n' reboot_sim; ntpd_stopped fire BOUND true @@ -200,17 +224,6 @@ reboot_sim; : > "$NTPD_PID" fire BOUND true mustnt "empty pidfile: does not kick" kicked -# The ordering case the hook was written around: on a wired box this fires -# during S41dhcpcd, before S49ntp has started ntpd. The pass must bail out -# WITHOUT spending the stamp, so a genuinely late interface still gets its kick. -printf '\n--- the wired-box ordering case ---\n' -reboot_sim; ntpd_stopped -fire BOUND true -mustnt "fires before ntpd exists: no kick" kicked -ntpd_running -fire BOUND true -must "a later address event still gets its one kick" kicked - printf '\n--- events that must do nothing ---\n' reboot_sim; ntpd_running fire BOUND false @@ -225,6 +238,10 @@ fire PREINIT true mustnt "PREINIT does not kick" kicked fire EXPIRE true mustnt "EXPIRE does not kick" kicked +fire RENEW true +mustnt "RENEW does not kick even with ntpd up and stamp unspent" kicked +fire REBIND true +mustnt "REBIND does not kick either" kicked mustnt "none of the above spent the stamp" stamped printf '\n--- missing init script ---\n' From a462f12e636fde528be09f5433d523c2bfb39efb Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 3 Sep 2026 16:26:56 -0500 Subject: [PATCH 3/3] review: verify the pidfile names ntpd, and reject pid 0 Second Copilot pass on #147. One of four comments was a real bug; the other three were a line count that is wrong. REAL: `kill -0` proved only that SOME process existed, not that it was ntpd. The pidfile cannot outlive a boot (/var/run -> /run, tmpfs), but a crashed ntpd within one boot leaves one behind, and a long-running box can wrap pid_max and reuse that pid for something unrelated. We would then "restart ntpd" -- and S49ntp's stop() is stock's, `start-stop-daemon -K -p "$PIDFILE"` with no -x, so it would SIGTERM that unrelated process before starting a daemon that was not running. This hook is the first automatic caller of that script (50-ntp.conf's service_condcommand never fires here), so the exposure is ours to avoid. Replaced with `start-stop-daemon -K -t -q -p "$NTPD_PID" -x /usr/sbin/ntpd`, which does liveness and identity in one probe, rather than the /proc/PID/exe read the review suggested -- it reuses a primitive already validated on both BusyBox vintages and needs no /proc parsing. Measured against a live decoy on ours (1.38.0) and stock's (1.33.1, from Linux_Image_creator_MiSTer's rootfs.tar.bz2 under qemu-arm), identically on both: no -x -> 0, `-x /usr/sbin/ntpd` -> 1, decoy's own name -> 0. The match is by NAME, not resolved path (a decoy whose exe was /usr/lib/.../sleep still matched -x /usr/bin/sleep), which is ample: the case being excluded is a reused pid that is not ntpd. Also real, and called out in the same comment: pid 0 was accepted. `kill -0 0` signals the whole process GROUP, and the start-stop-daemon probe was measured to SUCCEED on a pidfile containing 0 -- so the probe will not reject it and the hook has to. Now rejected explicitly alongside empty and non-numeric. Tests 34 -> 38. The two new pidfile states run REAL processes so identity is exercised rather than mocked: bash copies named `ntpd` and `notntpd` (bash because coreutils and busybox dispatch on argv[0] and refuse to run renamed; the trailing `:` in `-c 'sleep 60; :'` defeats bash's exec optimisation, which would otherwise leave comm as "sleep"). start-stop-daemon is stubbed in the sandbox, encoding exactly the semantics measured on both real binaries -- including the pid-0 quirk and the not-gated-on-`-q` stderr warning -- because the harness also runs under target busybox ash via qemu and nesting qemu is not portable. Mutation-checked: dropping -x fails 2 cases, dropping the 0 rejection fails 2 others. 38/38 on the host shell and on target BusyBox ash. NOT CHANGED: the three line-count comments. They ask for 46 added lines in etc/network/interfaces; the measured value is 47, and diffing the new file against c196569 (P2.3, byte-identical to stock) enumerates them -- 43 comment lines plus 4 code lines, 0 removals. The review's "42 comment lines (3-44)" omits file line 2, a bare `#`, which is genuinely added: stock's banner on line 1 is followed immediately by a blank line. Changing 47 to 46 would make three documents wrong, so they stand. The one valid half of that group is fixed: wifi-parity.md's cross-reference to docs/init-parity.md:147 was stale, and the row is now at :150. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014SBpBTbtBsFwjAgKCx5T9S --- .../usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick | 34 ++++++- docs/init-parity.md | 2 +- docs/wifi-parity.md | 2 +- scripts/ci-tests.sh | 6 +- scripts/test-ntp-kick.sh | 94 +++++++++++++++++-- 5 files changed, 121 insertions(+), 17 deletions(-) diff --git a/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick b/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick index b0b753a6..1b3c1a8d 100644 --- a/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick +++ b/board/mister/de10nano/rootfs-overlay/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick @@ -137,6 +137,9 @@ BOUND|REBOOT|BOUND6|REBOOT6) # moved PIDFILE, the check below simply fails and this hook does # nothing -- the safe direction. NTPD_PID=/var/run/ntpd.pid + # Matched against the pidfile's process so a stale pidfile whose + # pid has been REUSED cannot make us restart -- see below. + NTPD_EXEC=/usr/sbin/ntpd # tmpfs, so the once-per-boot stamp re-arms on every boot. STAMPDIR=/run/ntp-kick @@ -145,12 +148,35 @@ BOUND|REBOOT|BOUND6|REBOOT6) [ -x "$NTP_INIT" ] || exit 0 [ -s "$NTPD_PID" ] || exit 0 ntpd_pid="$(cat "$NTPD_PID" 2>/dev/null)" + # Reject empty, non-numeric, and 0. 0 matters on its own: `kill -0 + # 0` signals the whole process GROUP, and start-stop-daemon -t + # against a pidfile containing 0 was measured to return success + # on this image -- either would make a garbage pidfile look live. case "$ntpd_pid" in - ''|*[!0-9]*) exit 0 ;; + ''|0|*[!0-9]*) exit 0 ;; esac - # A stale pidfile cannot survive the tmpfs, but a crashed ntpd - # within one boot can leave one. kill -0 is the liveness test. - kill -0 "$ntpd_pid" 2>/dev/null || exit 0 + # Liveness AND identity, in one probe. Liveness alone is not + # enough: the pidfile cannot outlive a boot (tmpfs), but a crashed + # ntpd within one boot leaves one behind, and a long-running box + # can wrap pid_max and reuse that pid for something unrelated. + # We would then restart "ntpd" -- and S49ntp's stop() is stock's, + # which does `start-stop-daemon -K -p "$PIDFILE"` with no -x, so + # it would SIGTERM that unrelated process before starting a daemon + # that was not running. This hook is the first automatic caller of + # that script (50-ntp.conf's service_condcommand never fires here, + # see above), so the exposure is ours to avoid. + # + # -x makes start-stop-daemon match the process as well as the + # pidfile. Measured on both BusyBox vintages that matter -- ours + # (1.38.0) and stock's (1.33.1), the latter extracted from + # Linux_Image_creator_MiSTer's rootfs.tar.bz2 and run under + # qemu-arm -- against a live decoy process: without -x the probe + # returns 0, with `-x /usr/sbin/ntpd` it returns 1, and with the + # decoy's own name it returns 0. Note the match is by name, not by + # resolved path (a decoy whose exe was /usr/lib/.../sleep still + # matched `-x /usr/bin/sleep`), which is ample here: the case being + # excluded is a reused pid belonging to something that is not ntpd. + start-stop-daemon -K -t -q -p "$NTPD_PID" -x "$NTPD_EXEC" 2>/dev/null || exit 0 mkdir "$STAMPDIR" 2>/dev/null || exit 0 diff --git a/docs/init-parity.md b/docs/init-parity.md index 8a1d0921..a86c7e81 100644 --- a/docs/init-parity.md +++ b/docs/init-parity.md @@ -152,7 +152,7 @@ regardless, so the worst case is slow, never weak. | `/etc/inittab` | **adapted** (3 documented deviations) | Full stock shape reproduced (`::sysinit:/media/fat/MiSTer &`, `/etc/resync &`, `rcS`, shutdown sequence) with: **(1)** the remount-rw sysinit line kept **commented out**, exactly as stock has it — Buildroot's own skeleton default inittab ships this line **uncommented**, which would remount `/` rw at every sysinit and defeat the whole read-only-root design (ADR 0011); confirmed this project's own finalize hook tries to uncomment it too (see below) and is overridden by the overlay running last. **(2)** the serial console runs `ttyS0::respawn:/sbin/agetty --nohostname -L ttyS0 115200 vt100` — util-linux `agetty`, matching stock (whose inittab also uses `agetty --nohostname`), now that `BR2_PACKAGE_UTIL_LINUX_AGETTY` is enabled (see `docs/util-linux-parity.md`). It still targets `ttyS0` explicitly rather than stock's `console` alias, because this board's actual cmdline is `console=ttyS0,115200` (`docs/boot-chain.md`). agetty treats a numeric positional argument as the baud rate, so `ttyS0 115200` and `115200 ttyS0` are equivalent; the port-first order is kept for continuity with the previous BusyBox `getty` line. (Earlier revisions used BusyBox `getty` because `BR2_PACKAGE_UTIL_LINUX_BINARIES` was not selected — that constraint no longer holds; the overlapping BusyBox `getty` applet is now disabled so `agetty` is the console.) **(3)** `gpm` is invoked at `/usr/sbin/gpm`, not stock's `/sbin/gpm` — originally **a real bug caught during acceptance verification**: gpm is a real package (not a BusyBox applet, which lands under `/sbin` regardless of usr-merge), and when this row was first written (P2.3) the rootfs was **not** usr-merged — `/sbin` was a real directory, not `-> /usr/sbin` the way stock's `work/imgroot/sbin` is — so `/sbin/gpm` genuinely did not exist in the image. An inittab `sysinit` line is an **absolute path**, so unlike `/etc/profile`'s `PATH` it gets no shell search at all; it would have silently failed to spawn on every single boot. Caught by exhaustively grep'ing every absolute path referenced anywhere in the overlay against the actually-built image (see the task report) — first build had this wrong; fixed and rebuilt before acceptance. **Since P2.9 v2** (`7be9ee5`) set `BR2_ROOTFS_MERGED_USR=y`, that premise no longer holds: this build is now usr-merged like stock (verified: `output/target/sbin -> usr/sbin`), so `/sbin/gpm` *would* resolve today. **The deviation is kept anyway, deliberately.** Buildroot builds gpm with `--prefix=/usr`, so the binary's real path is `/usr/sbin/gpm` in *either* layout (gpm's own file list: `gpm,./usr/sbin/gpm`) — stock's binary is at that same physical path, and stock's inittab only reaches it through the `/sbin -> usr/sbin` compat symlink that exists solely because stock is usr-merged. So our path is correct in both layouts and stock's is correct in only one; ours survives an unmerge, stock's would break on one. Given P2.3 already got bitten by exactly that (the unmerged build had no `/sbin/gpm` at all), pointing at the real path rather than a merge-dependent alias is the more durable choice, even though a future unmerge is unlikely. This is an **accepted permanent deviation from stock**, not a cleanup waiting to happen. **Also dropped — later RESTORED, guarded (T3, 2026-07-27):** `loadkeys /etc/kbd.map` and `setfont` were dropped by P2.3 because this BusyBox build has no `loadkeys` applet at all and `CONFIG_SETFONT` is explicitly not set (still true — re-verified in `output/build/busybox-1.38.0/.config`; busybox's `loadkmap` applet is not a substitute, it reads binary bkeymap, not stock's text keymap); keeping either line verbatim would just fail every boot. T3 vendored stock's `etc/kbd.map` (it blanks the F12/Mute/Vol± keycodes Main_MiSTer consumes via evdev) and restored both lines wrapped in `[ -x /usr/bin/... ]` guards: with the parallel T5 task's `BR2_PACKAGE_KBD` (the same package stock's own loadkeys/setfont came from) the lines do stock's exact job, without it they are silent no-ops instead of boot errors. See the inittab's own note 3 and `docs/stock-reconciliation.md` §3c. `gpm -m /dev/input/mice -t imps2` itself **is** kept (just at the corrected path) — `BR2_PACKAGE_GPM=y` was deliberately selected in P2.1 for this, and the binary is present. | | `/etc/profile` | **adapted** (1 documented deviation) | Full stock content reproduced (PATH, `PS1='$(pwd)# '`, `EDITOR=/bin/vi`, `/etc/profile.d/*.sh` sourcing, `LC_ALL=en_US.UTF-8`, and critically the login-time `mount -o remount,rw /` — this is how `/` ever becomes writable at all, matching stock and ADR 0011's own description of the mechanism). **Deviation:** `PATH` gains an explicit `/bin:/sbin:` prefix stock's literal string doesn't have. Stock's rootfs is usr-merged (`work/imgroot`: `/bin -> usr/bin`, `/sbin -> usr/sbin`), so its `PATH="/usr/bin:/usr/sbin"` already covered `/bin`/`/sbin` for free. When this deviation was introduced (P2.3) *this* build was a plain (non-merged) skeleton layout, so omitting `/bin:/sbin` from `PATH` would have silently dropped most BusyBox applets from every interactive shell; changing `BR2_ROOTFS_MERGED_USR` was then out of P2.3's scope ("do NOT disturb ... the package set"), making the overlay `PATH` the correct-altitude fix. **Since P2.9 v2** (`7be9ee5`) set `BR2_ROOTFS_MERGED_USR=y` — the change that fixed the `/lib/security/pam_unix.so` SSH lockout — this build is usr-merged too, so the `/bin:/sbin:` prefix is now redundant. It is kept because it is harmless (the paths resolve to the same directories) and matches Buildroot's own skeleton default profile. The file's own header comment records the same history. | | `/usr/lib/dhcpcd/dhcpcd-hooks/90-timezone` | **added (divergence, [ADR 0025](decisions/0025-first-boot-timezone-autodetect.md))** | Stock has no equivalent, and that is the gap it closes: `/etc/localtime` points at `/media/fat/linux/timezone`, which **does not exist on a fresh card**, so glibc falls back to UTC silently and permanently. The first time dhcpcd brings an interface up with an address, this asks `ip-api.com` for the zone of the box's public IP and copies `/usr/share/zoneinfo/posix/` to that path — the *same* provider, destination and file format as the community `Scripts_MiSTer/timezone.sh` "Automatic" mode, so the two are interchangeable. No new package (`curl` and tzdata were already in the image). **Note this adds no init script**: an earlier revision had an `S48timezone` as well, but on a DHCP box the lease has usually not landed by S48, so it was near-redundant with this hook — and it carried a `/proc/net/route` check whose IPv6 arm silently matched the kernel's own `ip6_null_entry`. A static-IP box configured only in `/etc/network/interfaces` never runs dhcpcd and so never autodetects; accepted deliberately, since setting a static address is already a by-hand act. Properties worth stating because they are the design: the guess is spent **once, and only when it was actually made** (gated on the timezone file *and* on a `timezone.autodetect` stamp written only when a provider answered *with a zone name*, so neither being offline nor a captive portal's HTTP 200 burns it), it **never overwrites** a timezone anyone has already set (re-checked immediately before the write, not only at the gate), and it **delays nothing** — the body is a backgrounded subshell, which also keeps it from leaking a single variable or function into dhcpcd's shell. Sourced, not executed (`dhcpcd-run-hooks`: `. "$hook"`), hence no shebang, no exec bit, and no `exit` — an `exit` here would end dhcpcd's whole hook run and take `20-resolv.conf`/`30-hostname` with it. Zone names arrive off the network, so they are validated against the shipped zoneinfo before being used as a path; `scripts/test-timezone.sh` asserts each rejection, and mutation-checks the two that could otherwise pass vacuously. | -| `/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick` | **added (divergence)** | Stock has no equivalent. The gap: this board has no RTC, so a cold boot starts at the epoch, and `S49ntp` launches `ntpd -g` at a fixed point in `rcS` whether or not a network exists — which on the common path it does not, since WiFi association plus DHCP normally completes well after S49. ntpd survives that in one half and not the other. The **interface** half heals itself: ntpd is built with `HAVE_RTNETLINK` (`config.h:786`), so an address appearing is a netlink event and `ntp_io.c:1980` → `ntp_peer.c:763` `refresh_all_peerinterfaces` reattaches peers within ~3 s. The **DNS** half does not: for a `server ` line the peer is created *only* in the DNS callback (`ntp_config.c:4459-4463` → `peer_name_resolved`), and at S49 there is no `/etc/resolv.conf` — `20-resolv.conf` writes it in this very hook pass — so resolution fails and **no peers exist at all** for the rescan to reattach. The clock then waits on ntpd's DNS retry backoff and nothing else: `libntp/ntp_intres.c` `manage_dns_retry_interval` walks 2-3-4-6-8-12-16-24-32-48-64 s (`config.h:1313` leaves `IGNORE_DNS_ERRORS` undefined, so `DNSFLAGS` is 0 and `retmax` is 64, not 1024), putting attempts at t = 2, 5, 9, 15, 23, 35, 51, 75, 107, 155, 219, 283 … s after ntpd started. Measured cost of the next-slot wait: a few seconds on a wired box, 10-20 s on a typical WiFi boot, 30 s to a couple of minutes when the network turns up late. This restarts ntpd once, when an address actually arrives, so it re-resolves against the `resolv.conf` that now exists; `/etc/ntp.conf`'s `iburst` and `S49ntp`'s `-g` then give correct time within ~10-15 s. **Not `sntp`**: `BR2_PACKAGE_NTP_SNTP` would also install Buildroot's `S48sntp` (`package/ntp/ntp.mk:110-114`), which runs sntp at boot one script *before* ntpd with no network, and would have to be suppressed — not worth ~10 s. **Not `service_condcommand`** (which `50-ntp.conf` uses): dhcpcd's `detect_init` finds no systemctl/rc-service/invoke-rc.d/service/sv here and falls through to its `/etc/init.d` branch, which tests `[ -x /etc/init.d/ntpd ]` — ours is `S49ntp` — and `service_status` then runs `$x/$1 status`, a verb `S49ntp` has no case for; both fail silently. The hook calls the script directly. **Not a udev rule**: a `net` `add` uevent fires when the netdev is *created*, before association, lease, route or `resolv.conf`, which would reproduce the bug one layer down. Design properties: fires **at most once per boot** (`mkdir /run/ntp-kick` is the atomic test-and-set, on a tmpfs, so it re-arms each boot — two interfaces can `BOUND` at the same moment, and restarting ntpd more than once would discard its accumulated clock discipline for nothing); **never starts an ntpd that is not running**, so a by-hand or `/etc/default/ntpd` disable stands; and the stamp is claimed **last**, after every gate, so a pass that bails out leaves the one kick available — which is what makes the wired-box ordering case correct, where `S41dhcpcd` blocks until the first lease and so fires this hook *before* `S49ntp` has started ntpd at all. Backgrounded subshell (`S49ntp restart` contains a `sleep 1`), sourced not executed — no shebang, no exec bit, no `exit`. A static-IP box never runs dhcpcd and never gets kicked; accepted, the same gap ADR 0025 accepts, and ntpd's own backoff still converges there. **Acquisition reasons only** — `BOUND`/`REBOOT` and their `*6` forms, deliberately *not* `RENEW`/`REBIND` as `90-timezone` matches: dhcpcd picks those two only when `state->old` is non-NULL (`src/dhcp.c:2499-2513`), i.e. the address was already there. Matching them combines badly with the re-armable stamp — a renewal arriving hours into the session would restart an ntpd that had been synchronised the whole time, and the wired ordering case makes that reachable rather than theoretical, since it always bails out first and leaves the kick armed. Nothing is lost: WiFi associating late, a dongle plugged in later and an AP returning are all *new* leases, hence `BOUND`; and an address that merely changes under a `RENEW`/`REBIND` needs no kick, because ntpd holds peers by then and the `HAVE_RTNETLINK` path reattaches them in ~3 s. (Found in review of PR #147.) `scripts/test-ntp-kick.sh` asserts the behaviour in 34 sandboxed cases (run twice by `ci-tests.sh`: host shell, and the target's own BusyBox ash under qemu-arm), mutation-checked against dropping the once-per-boot gate, the liveness check, and the narrowed reason set. | +| `/usr/lib/dhcpcd/dhcpcd-hooks/91-ntp-kick` | **added (divergence)** | Stock has no equivalent. The gap: this board has no RTC, so a cold boot starts at the epoch, and `S49ntp` launches `ntpd -g` at a fixed point in `rcS` whether or not a network exists — which on the common path it does not, since WiFi association plus DHCP normally completes well after S49. ntpd survives that in one half and not the other. The **interface** half heals itself: ntpd is built with `HAVE_RTNETLINK` (`config.h:786`), so an address appearing is a netlink event and `ntp_io.c:1980` → `ntp_peer.c:763` `refresh_all_peerinterfaces` reattaches peers within ~3 s. The **DNS** half does not: for a `server ` line the peer is created *only* in the DNS callback (`ntp_config.c:4459-4463` → `peer_name_resolved`), and at S49 there is no `/etc/resolv.conf` — `20-resolv.conf` writes it in this very hook pass — so resolution fails and **no peers exist at all** for the rescan to reattach. The clock then waits on ntpd's DNS retry backoff and nothing else: `libntp/ntp_intres.c` `manage_dns_retry_interval` walks 2-3-4-6-8-12-16-24-32-48-64 s (`config.h:1313` leaves `IGNORE_DNS_ERRORS` undefined, so `DNSFLAGS` is 0 and `retmax` is 64, not 1024), putting attempts at t = 2, 5, 9, 15, 23, 35, 51, 75, 107, 155, 219, 283 … s after ntpd started. Measured cost of the next-slot wait: a few seconds on a wired box, 10-20 s on a typical WiFi boot, 30 s to a couple of minutes when the network turns up late. This restarts ntpd once, when an address actually arrives, so it re-resolves against the `resolv.conf` that now exists; `/etc/ntp.conf`'s `iburst` and `S49ntp`'s `-g` then give correct time within ~10-15 s. **Not `sntp`**: `BR2_PACKAGE_NTP_SNTP` would also install Buildroot's `S48sntp` (`package/ntp/ntp.mk:110-114`), which runs sntp at boot one script *before* ntpd with no network, and would have to be suppressed — not worth ~10 s. **Not `service_condcommand`** (which `50-ntp.conf` uses): dhcpcd's `detect_init` finds no systemctl/rc-service/invoke-rc.d/service/sv here and falls through to its `/etc/init.d` branch, which tests `[ -x /etc/init.d/ntpd ]` — ours is `S49ntp` — and `service_status` then runs `$x/$1 status`, a verb `S49ntp` has no case for; both fail silently. The hook calls the script directly. **Not a udev rule**: a `net` `add` uevent fires when the netdev is *created*, before association, lease, route or `resolv.conf`, which would reproduce the bug one layer down. Design properties: fires **at most once per boot** (`mkdir /run/ntp-kick` is the atomic test-and-set, on a tmpfs, so it re-arms each boot — two interfaces can `BOUND` at the same moment, and restarting ntpd more than once would discard its accumulated clock discipline for nothing); **never starts an ntpd that is not running**, so a by-hand or `/etc/default/ntpd` disable stands; and the stamp is claimed **last**, after every gate, so a pass that bails out leaves the one kick available — which is what makes the wired-box ordering case correct, where `S41dhcpcd` blocks until the first lease and so fires this hook *before* `S49ntp` has started ntpd at all. Backgrounded subshell (`S49ntp restart` contains a `sleep 1`), sourced not executed — no shebang, no exec bit, no `exit`. A static-IP box never runs dhcpcd and never gets kicked; accepted, the same gap ADR 0025 accepts, and ntpd's own backoff still converges there. **Acquisition reasons only** — `BOUND`/`REBOOT` and their `*6` forms, deliberately *not* `RENEW`/`REBIND` as `90-timezone` matches: dhcpcd picks those two only when `state->old` is non-NULL (`src/dhcp.c:2499-2513`), i.e. the address was already there. Matching them combines badly with the re-armable stamp — a renewal arriving hours into the session would restart an ntpd that had been synchronised the whole time, and the wired ordering case makes that reachable rather than theoretical, since it always bails out first and leaves the kick armed. Nothing is lost: WiFi associating late, a dongle plugged in later and an AP returning are all *new* leases, hence `BOUND`; and an address that merely changes under a `RENEW`/`REBIND` needs no kick, because ntpd holds peers by then and the `HAVE_RTNETLINK` path reattaches them in ~3 s. (Found in review of PR #147.) `scripts/test-ntp-kick.sh` asserts the behaviour in 38 sandboxed cases (run twice by `ci-tests.sh`: host shell, and the target's own BusyBox ash under qemu-arm), mutation-checked against dropping the once-per-boot gate, the liveness check, the narrowed reason set, the process-identity check and the pid-0 rejection. The liveness probe is `start-stop-daemon -K -t -q -p -x /usr/sbin/ntpd`, which checks identity as well: the pidfile cannot outlive a boot (tmpfs), but a crashed ntpd within one boot leaves one, and a long-running box can wrap `pid_max` and reuse that pid — restarting then would have `S49ntp`'s stock `stop()` (`start-stop-daemon -K -p` with no `-x`) SIGTERM an unrelated process, and this hook is the first automatic caller of that script. A pidfile containing `0` is rejected explicitly, because `kill -0 0` signals the process group and the probe itself was measured to succeed on it. Both behaviours measured on ours (BusyBox 1.38.0) and stock's (1.33.1). (Found in review of PR #147.) | | `/usr/lib/dhcpcd/dhcpcd-hooks/` (the set) | **pinned (build-host independence)** | dhcpcd's `configure` chooses which hooks to install by probing the **build host** for `ntpd`/`chronyd`/`systemd-timesyncd`/`ypbind`, and Buildroot's `dhcpcd.mk` passes no `--with-hooks`, so the image inherited whatever daemons the build machine happened to have: a GitHub runner produced `50-ntp.conf`, a developer box with timesyncd produced `50-timesyncd.conf` — with a green build both times (found 2026-09-02 by diffing a CI image against a local one). `external.mk` now appends `--with-hooks=ntp.conf --with-eghooks=yp.conf` to `DHCPCD_CONFIG_OPTS`, which reproduces the canonical CI image exactly (`01-test`, `20-resolv.conf`, `30-hostname`, `50-ntp.conf`, plus our `90-timezone` and `91-ntp-kick`; `50-yp.conf` stays an example under `/usr/share/dhcpcd/hooks`), and `scripts/check-linux-img.sh` asserts that exact set. `50-ntp.conf` is the right hook to pin given we ship classic `ntpd` (stock parity), though it is **inert in this image**: it merges DHCP-offered servers into `/etc/ntp.conf`, and `etc/dhcpcd.conf` leaves `option ntp_servers` commented out exactly as stock does, so it is never offered any. It is pinned because its *presence* is what proves `configure` did not probe the build host — not because it runs. Its `service_condcommand ntpd restart` could not work here either; see the `91-ntp-kick` row. Upstream-worthy: Buildroot's `dhcpcd.mk` should pass `--with-hooks` itself. | | `/etc/resync` | **identical** | Byte-for-byte match (53 bytes: `( while [ 1 ]; do sync; sleep 5; done ) &`). Executable bit set. | | `/etc/proftpd.conf` | **identical, with a flagged security caveat** | Byte-for-byte reproduction of stock: `User root`/`Group root`, `RootLogin on`, anonymous ` AllowAll`, `Umask 000`. **Note, not acted on:** the package's own default `proftpd.conf` is meaningfully more hardened (`User nobody`, no root login, anonymous write denied). This project's task scope names exactly two sanctioned security improvements (resolv.conf's upstream default, ADR 0015's SSH host keys); silently hardening FTP as a third, undocumented one would change a well-known MiSTer workflow (anonymous/root FTP to `/media/fat`) without a maintainer decision or its own ADR. Shipped as stock parity; flagged here and in the task report as a candidate for a future ADR, not decided unilaterally. | diff --git a/docs/wifi-parity.md b/docs/wifi-parity.md index ceda78f5..338fbaf4 100644 --- a/docs/wifi-parity.md +++ b/docs/wifi-parity.md @@ -98,7 +98,7 @@ path (stock's own README-level instructions) both depend on. | Contract element | Stock (`work/imgroot`) | Ours | Status | |---|---|---|---| -| `/etc/network/interfaces` | `wlan0`/`wlan1` `iface … inet manual` with `pre-up wpa_supplicant -s -B -P /run/wpa_supplicant.$IFACE.pid -i $IFACE -D nl80211,wext -c /media/fat/linux/wpa_supplicant.conf`, `post_up sleep 2`, `post-down killall -q wpa_supplicant` | `board/mister/de10nano/rootfs-overlay/etc/network/interfaces` | **Adapted (v9, revised).** Stock's content, plus a header comment and two `pre-up` lines per `wlan` stanza: a `[ -e /sys/class/net/$IFACE ]` device-presence guard and the `i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info …` wait loop behind it — `diff` against `work/imgroot/etc/network/interfaces` exits 1 with exactly 47 added lines (43 comment + 4 code) and zero removals. Every stock directive is reproduced unchanged; nothing is removed. Authored by P2.3 (then byte-identical), diverged by `4cf2fc7` (v9); the guard was added later to stop `S40network`'s `ifup -a` burning the full 20 s on an absent `wlan1` (§9). `docs/init-parity.md:147` carries the same row. | +| `/etc/network/interfaces` | `wlan0`/`wlan1` `iface … inet manual` with `pre-up wpa_supplicant -s -B -P /run/wpa_supplicant.$IFACE.pid -i $IFACE -D nl80211,wext -c /media/fat/linux/wpa_supplicant.conf`, `post_up sleep 2`, `post-down killall -q wpa_supplicant` | `board/mister/de10nano/rootfs-overlay/etc/network/interfaces` | **Adapted (v9, revised).** Stock's content, plus a header comment and two `pre-up` lines per `wlan` stanza: a `[ -e /sys/class/net/$IFACE ]` device-presence guard and the `i=0; while [ $i -lt 20 ] && ! iw dev $IFACE info …` wait loop behind it — `diff` against `work/imgroot/etc/network/interfaces` exits 1 with exactly 47 added lines (43 comment + 4 code) and zero removals. Every stock directive is reproduced unchanged; nothing is removed. Authored by P2.3 (then byte-identical), diverged by `4cf2fc7` (v9); the guard was added later to stop `S40network`'s `ifup -a` burning the full 20 s on an absent `wlan1` (§9). `docs/init-parity.md:150` carries the same row. | | `/etc/init.d/S40network` | `ifup -a` / `ifdown -a` (ifupdown-scripts package default) | Not overlaid — `BR2_PACKAGE_IFUPDOWN_SCRIPTS`'s own Kconfig default (`default y if BR2_ROOTFS_SKELETON_DEFAULT`, `work/buildroot/package/ifupdown-scripts/Config.in`) auto-selects it; our defconfig sets neither `BR2_PACKAGE_SYSTEMD_NETWORKD` nor `BR2_PACKAGE_NETIFRC` (the two symbols that would suppress it) and leaves `BR2_ROOTFS_SKELETON_DEFAULT` at Buildroot's own default (y) | **Identical**, confirmed byte-for-byte by P2.3 (`docs/init-parity.md:63`); re-confirmed the selecting conditions still hold in this defconfig. | | `/etc/init.d/S41dhcpcd` | starts `dhcpcd` globally (no `-i`) | Package default, not overlaid; `BR2_PACKAGE_DHCPCD=y` (defconfig line 807, P2.1) | **Functionally identical** (P2.3 finding, `docs/init-parity.md:64`) — only the PID-file path differs, an artifact of the newer dhcpcd release, not a decision point. | | `/etc/dhcpcd.conf` | `hostname`, `clientid`, `option rapid_commit`, etc. enabled | `board/mister/de10nano/rootfs-overlay/etc/dhcpcd.conf` | **Identical.** `diff` exit 0 (re-verified this task). Authored by P2.3. | diff --git a/scripts/ci-tests.sh b/scripts/ci-tests.sh index 6b932fce..4cad99be 100755 --- a/scripts/ci-tests.sh +++ b/scripts/ci-tests.sh @@ -1803,11 +1803,11 @@ fi # that is not running, not spending its one kick on a pass that bailed out, and # the two SOURCED properties -- has its own sandboxed harness. No build, no # board, no network. -printf -- '--- test-ntp-kick.sh: ntpd kick hook behaviour (34 cases) ---\n' +printf -- '--- test-ntp-kick.sh: ntpd kick hook behaviour (38 cases) ---\n' if "$ROOT/scripts/test-ntp-kick.sh"; then - pass "test-ntp-kick.sh (ntpd kick hook behaviour, 34 cases)" + pass "test-ntp-kick.sh (ntpd kick hook behaviour, 38 cases)" else - fail "test-ntp-kick.sh (ntpd kick hook behaviour, 34 cases)" \ + fail "test-ntp-kick.sh (ntpd kick hook behaviour, 38 cases)" \ "one or more cases failed -- see output above" fi diff --git a/scripts/test-ntp-kick.sh b/scripts/test-ntp-kick.sh index df17e6d1..82682270 100755 --- a/scripts/test-ntp-kick.sh +++ b/scripts/test-ntp-kick.sh @@ -70,12 +70,68 @@ fi read -r -a TEST_SH <<< "${NTP_TEST_SH:-sh}" SB="$(mktemp -d "${TMPDIR:-/tmp}/ntp-kick-test.XXXXXX")" -trap 'rm -rf "$SB"' EXIT +mkdir -p "$SB/bin" +trap 'pkill -P $$ -f "$SB/(not)?ntpd" 2>/dev/null; rm -rf "$SB"' EXIT NTP_INIT="$SB/S49ntp" NTPD_PID="$SB/ntpd.pid" STAMPDIR="$SB/ntp-kick" CALLS="$SB/init.calls" +NTPD_EXEC="$SB/ntpd" + +# Real executables named "ntpd" and "notntpd", so /proc/PID/comm makes the +# identity check testable with live processes rather than mocks. bash is the +# copy source because this script already requires it, and unlike coreutils or +# busybox it does not dispatch on argv[0] and refuse to run under another name. +# The trailing `:` in the command defeats bash's exec optimisation for a single +# command -- without it bash exec()s sleep and comm becomes "sleep", not "ntpd". +cp "$BASH" "$NTPD_EXEC" +cp "$BASH" "$SB/notntpd" + +# Stub for busybox's start-stop-daemon probe. It encodes exactly the semantics +# MEASURED on the two binaries that matter -- ours (BusyBox 1.38.0) and stock's +# (1.33.1, extracted from Linux_Image_creator_MiSTer's rootfs.tar.bz2 and run +# under qemu-arm) -- rather than guessing them: +# live pid, no -x -> 0 +# live pid, -x naming another process -> 1 +# live pid, -x naming this process -> 0 (matched by NAME, not by +# resolved path: a decoy whose +# exe was /usr/lib/.../sleep +# still matched -x /usr/bin/sleep) +# reaped pid -> 1, and prints "warning: killing +# process N: No such process" to +# stderr, NOT gated on -q, on BOTH +# pidfile containing 0 -> 0 (the quirk the hook must reject +# for itself) +# A stub rather than the real ARM binary because the harness also runs under +# target busybox ash via qemu, and nesting qemu inside qemu to reach the real +# one is not portable. The real binary's behaviour is pinned by the measurement +# above, recorded in docs/bluetooth-parity.md's sibling analysis and the hook. +cat > "$SB/bin/start-stop-daemon" <<'EOSSD' +#!/bin/sh +pidfile=""; want="" +while [ $# -gt 0 ]; do + case "$1" in + -p) pidfile="$2"; shift 2 ;; + -x) want="$2"; shift 2 ;; + *) shift ;; + esac +done +[ -n "$pidfile" ] && [ -s "$pidfile" ] || exit 1 +pid=$(cat "$pidfile") +case "$pid" in ''|*[!0-9]*) exit 1 ;; esac +[ "$pid" = 0 ] && exit 0 +if ! kill -0 "$pid" 2>/dev/null; then + echo "start-stop-daemon: warning: killing process $pid: No such process" >&2 + exit 1 +fi +if [ -n "$want" ]; then + comm=$(cat "/proc/$pid/comm" 2>/dev/null) + [ "$comm" = "${want##*/}" ] || exit 1 +fi +exit 0 +EOSSD +chmod +x "$SB/bin/start-stop-daemon" # The stub stands in for /etc/init.d/S49ntp: record the verb, say nothing. cat > "$NTP_INIT" </dev/null # reboot — a fresh boot: tmpfs stamp gone, no record of past calls. reboot_sim() { rm -rf "$STAMPDIR"; rm -f "$CALLS"; } -# ntpd_running / ntpd_stopped / ntpd_stale — the three pidfile states. -ntpd_running() { printf '%s\n' "$$" > "$NTPD_PID"; } -ntpd_stopped() { rm -f "$NTPD_PID"; } -ntpd_stale() { printf '%s\n' "$DEADPID" > "$NTPD_PID"; } +# Pidfile states. The live ones start a REAL process with the right (or wrong) +# name, so the hook's identity probe is exercised rather than mocked away. +_daemon="" +_stop_daemon() { [ -n "$_daemon" ] && kill "$_daemon" 2>/dev/null; wait "$_daemon" 2>/dev/null; _daemon=""; } +ntpd_running() { _stop_daemon; "$NTPD_EXEC" -c 'sleep 60; :' & _daemon=$!; printf '%s\n' "$_daemon" > "$NTPD_PID"; } +ntpd_impostor() { _stop_daemon; "$SB/notntpd" -c 'sleep 60; :' & _daemon=$!; printf '%s\n' "$_daemon" > "$NTPD_PID"; } +ntpd_stopped() { _stop_daemon; rm -f "$NTPD_PID"; } +ntpd_stale() { _stop_daemon; printf '%s\n' "$DEADPID" > "$NTPD_PID"; } # fire [reason] [if_up] -- one dhcpcd address event, sourced the way # dhcpcd-run-hooks sources it, INSIDE ${TEST_SH[@]}. Running it under the @@ -149,11 +210,11 @@ fire() { # shellcheck disable=SC2016 # $1/$2 belong to the inner shell, on purpose _src='. "$1"; : > "$2"' if [ "${2-}" = unset ]; then - env -u if_up reason="${1:-BOUND}" \ + env -u if_up PATH="$SB/bin:$PATH" reason="${1:-BOUND}" \ "${TEST_SH[@]}" -c "$_src" _ \ "$SB/sync" "$SB/returned-to-dhcpcd" > "$SB/out" 2>&1 else - env reason="${1:-BOUND}" if_up="${2-true}" \ + env PATH="$SB/bin:$PATH" reason="${1:-BOUND}" if_up="${2-true}" \ "${TEST_SH[@]}" -c "$_src" _ \ "$SB/sync" "$SB/returned-to-dhcpcd" > "$SB/out" 2>&1 fi @@ -224,6 +285,23 @@ reboot_sim; : > "$NTPD_PID" fire BOUND true mustnt "empty pidfile: does not kick" kicked +# Liveness alone is not enough. The pidfile cannot outlive a boot (tmpfs), but a +# crashed ntpd within one boot leaves one, and a long-running box can wrap +# pid_max and reuse that pid. S49ntp's stop() is stock's -- `start-stop-daemon +# -K -p "$PIDFILE"` with no -x -- so restarting on a reused pid would SIGTERM an +# unrelated process. Found in review of PR #147. +reboot_sim; ntpd_impostor +fire BOUND true +mustnt "reused pid owned by another process: does not kick" kicked +mustnt "reused pid: the stamp is NOT spent" stamped +# 0 is its own case: `kill -0 0` signals the process GROUP, and the real +# start-stop-daemon probe was measured to SUCCEED on a pidfile containing 0. +# The hook has to reject it itself -- the probe will not do it. +reboot_sim; ntpd_stopped; printf '0\n' > "$NTPD_PID" +fire BOUND true +mustnt "pidfile containing 0: does not kick" kicked +mustnt "pidfile 0: the stamp is NOT spent" stamped + printf '\n--- events that must do nothing ---\n' reboot_sim; ntpd_running fire BOUND false @@ -264,7 +342,7 @@ leak_check='. "$1"; for v in NTP_INIT NTPD_PID STAMPDIR ntpd_pid; do eval "val=\${$v-UNSET}"; [ "$val" = UNSET ] || { echo "LEAKED $v"; exit 1; } done; exit 0' reboot_sim; ntpd_running -if env reason=BOUND if_up=true "${TEST_SH[@]}" -c "$leak_check" _ "$SB/sync" \ +if env PATH="$SB/bin:$PATH" reason=BOUND if_up=true "${TEST_SH[@]}" -c "$leak_check" _ "$SB/sync" \ > "$SB/out" 2>&1; then ok "leaks no variables into dhcpcd's shell" else