boot-success forgets the firmware entries before it removes or sets aside the trial record

build/service-scripts/boot-success.sh

@@ -17,6 +17,8 @@ ident() { sed -n "s/^$1=//p" "$RUN/boot-identity" 2>/dev/null | head -1; }
 other_slot() { case "$1" in a) echo b ;; b) echo a ;; esac; }
 slot="$(ident slot)"; media="$(ident media)"; state="$(ident state)"
 now() { date -Iseconds 2>/dev/null || date; }
+# Drop FILE's clean pages, so the next read is what the device holds; best effort.
+uncache() { dd if="$1" iflag=nocache count=0 status=none 2>/dev/null || :; }   # uncache FILE
 result() { printf '%s %s\n' "$*" "$(now)" > "$B/last-result.new" && mv -f "$B/last-result.new" "$B/last-result"; }
 
 if [ -n "$media" ]; then
@@ -72,8 +74,9 @@ commit_slot() {   # commit_slot <slot>: make BOOTX64.EFI this slot's kernel
         if cmp -s "$src" "$dst"; then
             say "BOOTX64.EFI already is slot $1"; rc=0
         else
-            # Copy, fsync, then rename: on FAT only the rename is not atomic.
-            cp "$src" "$dst.new" && sync -f "$dst.new" && mv -f "$dst.new" "$dst" && sync -f "$dst" && rc=0
+            # Copy, fsync, compare as the device holds it, then rename: on FAT only the
+            # rename is not atomic, and a copy that landed wrong never becomes the boot file.
+            cp "$src" "$dst.new" && sync -f "$dst.new" && uncache "$dst.new" && cmp -s "$src" "$dst.new" && mv -f "$dst.new" "$dst" && sync -f "$dst" && rc=0
             [ "$rc" -eq 0 ] && say "committed: BOOTX64.EFI is now slot $1"
         fi
         # The record is part of the commit, and follows the boot file: kryptik-update
@@ -93,6 +96,8 @@ commit_slot() {   # commit_slot <slot>: make BOOTX64.EFI this slot's kernel
 
 # However a trial ends: a stale slot entry would outrank BOOTX64.EFI at every cold boot,
 # and the committed slot's own entry is a second way to it should that file be lost.
+# It runs before the trial record is removed or set aside: while that stands
+# kryptik-update arms nothing, so no BootNext an apply has just set is forgotten here.
 forget_entries() {   # forget_entries COMMITTED-SLOT
     if ! kryptik-efiboot forget >/dev/null 2>&1; then
         say "the firmware's Kryptik entries could not be removed; its own boot order may not name the committed slot"
@@ -134,9 +139,9 @@ if [ -n "$trial" ]; then
         failures="$(health)"
         if [ -z "$failures" ]; then
             if commit_slot "$slot"; then
+                forget_entries "$slot"
                 rm -f "$B/trial"
                 result "commit $slot"
-                forget_entries "$slot"
                 say "slot $slot is healthy and committed"
                 # Its release's signed date becomes the clock's floor if it is the newest (docs/design/time.md).
                 say "$(kryptikd time committed "$B/release-$slot" 2>&1)"
@@ -149,9 +154,10 @@ if [ -n "$trial" ]; then
             printf '%s\n' "$failures" | sed 's/^/boot-success:   - /'
             printf 'trial-unhealthy %s: %s\n' "$slot" "$(printf '%s' "$failures" | tr '\n' ';')" > "$B/last-result.new" \
                 && mv -f "$B/last-result.new" "$B/last-result"
+            forgot=1; forget_entries "$(other_slot "$slot")" || forgot=0
             [ ! -f "$B/trial" ] || mv -f "$B/trial" "$B/trial.failed"
             sync
-            if ! forget_entries "$(other_slot "$slot")" && [ -n "$unrecorded" ]; then
+            if [ "$forgot" = 0 ] && [ -n "$unrecorded" ]; then
                 # No trial record: only removing its entries stops the next boot repeating it.
                 say "not rebooting: with its entries still there the firmware could boot this trial again"
             elif [ "${KRYPTIK_NO_REBOOT:-0}" = 1 ]; then
@@ -168,13 +174,13 @@ if [ -n "$trial" ]; then
             # trial.failed stops kryptik-update re-arming this payload without --retry.
             say "trial slot $trial did NOT boot; running slot $slot again"
             result "trial-failed $trial"
-            mv -f "$B/trial" "$B/trial.failed"
             forget_entries "$slot"
+            mv -f "$B/trial" "$B/trial.failed"
         else
             say "the arming of slot $trial was interrupted before BootNext was set; nothing was tried"
             result "arming-interrupted $trial"
-            rm -f "$B/trial"
             forget_entries "$slot"
+            rm -f "$B/trial"
         fi
     fi
 elif [ -n "$stray" ]; then

docs/design/boot-and-updates.md

@@ -150,19 +150,19 @@ payloads come from [the update channel](update-channel.md) or by hand.
 5. `boot-success` judges the trial slot: state persistent; eudev, seatd, the
    launch daemon, the net zone and the login getty up; kryptikd finding kernel
    support and the zones; an unambiguous ESP. Healthy: it copies the kernel
-   over `BOOTX64.EFI` (`.new`, fsync, rename), updates `committed-slot` alike,
-   clears the trial and forgets the entries, and the slot's kept manifest
-   raises the clock's floor if it is the newest. Unhealthy: it records that,
-   forgets the entries and reboots into the committed slot, `BootNext` being
-   spent. On a degraded state the trial record is out of reach, so
-   `committed-slot` says whether the boot is a trial. A trial that never comes
-   up also lands on the committed slot; the fallback records `trial.failed`
-   and forgets the entries, and the updater will not re-arm that payload
-   without `--retry`, which root gives (`su` from the administration login
-   on tty2; the refusal prints the command). Only a trial reboots; an
-   unhealthy committed slot is reported and left running, and so is a trial
-   on a degraded state whose ESP cannot be read, since nothing then says it
-   is one.
+   over `BOOTX64.EFI` (`.new`, fsync, compare, rename), updates
+   `committed-slot` alike, clears the trial and forgets the entries, and the
+   slot's kept manifest raises the clock's floor if it is the newest.
+   Unhealthy: it records that, forgets the entries and reboots into the
+   committed slot, `BootNext` being spent. On a degraded state the trial
+   record is out of reach, so `committed-slot` says whether the boot is a
+   trial. A trial that never comes up also lands on the committed slot; the
+   fallback records `trial.failed` and forgets the entries, and the updater
+   will not re-arm that payload without `--retry`, which root gives (`su` from
+   the administration login on tty2; the refusal prints the command). Only a
+   trial reboots; an unhealthy committed slot is reported and left running,
+   and so is a trial on a degraded state whose ESP cannot be read, since
+   nothing then says it is one.
 
    The net zone is in the check on purpose: a release whose net zone cannot
    come up could never fetch the release that fixes it. Whoever can crash

tools/tests/boot-success.sh

@@ -37,6 +37,8 @@ EOF
 cat > "$T/bin/kryptik-efiboot" <<'EOF'
 #!/bin/sh
 echo "efiboot $*" >> "$KTEST/calls"
+# Whether the trial record still stood when the entries were forgotten.
+[ "$1" = forget ] && { [ -e "$KTEST/boot/trial" ] && echo kept || echo gone; } >> "$KTEST/forget-saw"
 [ ! -e "$KTEST/efiboot_fails" ] && [ ! -e "$KTEST/efiboot_fails_$1" ]
 EOF
 cat > "$T/bin/reboot" <<'EOF'
@@ -71,6 +73,13 @@ case "$*" in
     *BOOTX64.EFI.new) [ ! -e "$KTEST/sync_fails_boot" ] ;;
 esac
 EOF
+# cmp: the real one, but a copy of a kernel to BOOTX64.EFI.new can be made to read back wrong.
+REAL_CMP="$(command -v cmp)"
+cat > "$T/bin/cmp" <<EOF
+#!/bin/sh
+case "\$*" in *BOOTX64.EFI.new) [ -e "\$KTEST/copy_bad" ] && exit 1 ;; esac
+exec "$REAL_CMP" "\$@"
+EOF
 chmod +x "$T"/bin/*
 
 run_case() {   # run_case NAME SLOT MEDIA STATE TRIAL-CONTENT SERVICES...: stage a case
@@ -131,13 +140,17 @@ check "BOOTX64.EFI is now the slot b kernel" "$(cat "$KTEST/esp/EFI/BOOT/BOOTX64
 check "committed-slot records b" "$(cat "$KTEST/esp/kryptik/committed-slot")" "b"
 check "the new committed-slot record is fsynced under its temporary name" "$(grep -c -x "sync -f $KTEST/run/esp/kryptik/committed-slot.new" "$KTEST/syncs" 2>/dev/null)" "1"
 check "the trial record is gone" "$([[ -e "$KTEST/boot/trial" ]] && echo present || echo gone)" "gone"
+check "the entries are forgotten while the trial record still stands, so no apply arms in between" "$(cat "$KTEST/forget-saw" 2>/dev/null)" "kept"
 check "the trial's firmware entries and BootNext are forgotten after the commit, the committed slot gets its own, and its release goes to the clock's floor" "$CALLS" "mount -t vfat -o rw,nosuid,nodev,noexec /dev/vda1 $KTEST/run/esp umount $KTEST/run/esp efiboot forget efiboot ensure b kryptikd time committed $KTEST/boot/release-b "
 run_case recfail b "" persistent 'b\narmed=1\n' $ALL; : > "$KTEST/sync_fails"; go
 check "a committed-slot record that cannot be written fails the commit: the trial stays, for the next boot to commit again" \
     "$RESULT|$(cat "$KTEST/esp/kryptik/committed-slot")|$([[ -e "$KTEST/boot/trial" ]] && echo kept || echo gone)|$(grep -c 'efiboot forget' "$KTEST/calls" 2>/dev/null)" "commit-failed b|a|kept|0"
 run_case bootfail b "" persistent 'b\narmed=1\n' $ALL; : > "$KTEST/sync_fails_boot"; go
 check "a boot file that cannot be replaced fails the commit, and the record goes on naming the slot BOOTX64.EFI boots" \
     "$RESULT|$(cat "$KTEST/esp/EFI/BOOT/BOOTX64.EFI")|$(cat "$KTEST/esp/kryptik/committed-slot")|$([[ -e "$KTEST/boot/trial" ]] && echo kept || echo gone)" "commit-failed b|kernel-a|a|kept"
+run_case copybad b "" persistent 'b\narmed=1\n' $ALL; : > "$KTEST/copy_bad"; go
+check "a boot file whose copy reads back wrong is not renamed into place: the commit fails, the trial stays" \
+    "$RESULT|$(cat "$KTEST/esp/EFI/BOOT/BOOTX64.EFI")|$(cat "$KTEST/esp/kryptik/committed-slot")|$([[ -e "$KTEST/boot/trial" ]] && echo kept || echo gone)" "commit-failed b|kernel-a|a|kept"
 run_case commit0 b "" persistent 'b\narmed=0\n' $ALL; go
 check "a trial that booted before its armed=1 line was written is still a trial: committed" "$RESULT" "commit b"
 
@@ -154,6 +167,7 @@ run_case unzones b "" persistent 'b\narmed=1\n' $ALL; rm -f "$KTEST/zones_ok"; g
 check "trial whose zones do not load: not committed" "${RESULT%%:*}" "trial-unhealthy b"
 run_case unforget b "" persistent 'b\narmed=1\n' eudev; go
 check "an unhealthy trial forgets its entries, gives the committed slot its own, then reboots" "$CALLS" "efiboot forget efiboot ensure a reboot "
+check "an unhealthy trial's entries go before its record is set aside" "$(cat "$KTEST/forget-saw" 2>/dev/null)" "kept"
 run_case unforget2 b "" persistent 'b\narmed=1\n' eudev; : > "$KTEST/efiboot_fails"; go
 check "... and reboots if they stay: its record, now trial.failed, keeps it from coming back" "$(reboots)" "1"
 # On a degraded state /var is a tmpfs, so the trial record is out of reach.
@@ -181,9 +195,11 @@ check "back on the old slot with BootNext consumed: trial-failed" "$RESULT" "tri
 check "the record moved to trial.failed" "$([[ -e "$KTEST/boot/trial.failed" ]] && cat "$KTEST/boot/trial.failed" | head -1)" "b"
 check "BOOTX64.EFI untouched" "$(cat "$KTEST/esp/EFI/BOOT/BOOTX64.EFI")" "kernel-a"
 check "the failed trial's entries are forgotten, and the committed slot gets its own" "$CALLS" "efiboot forget efiboot ensure a "
+check "... before its record is set aside" "$(cat "$KTEST/forget-saw" 2>/dev/null)" "kept"
 run_case interrupted a "" persistent 'b\narmed=0\n' $ALL; go
 check "old slot with an armed=0 record: arming was interrupted, nothing failed" "$RESULT" "arming-interrupted b"
 check "the interrupted arming's entry is forgotten, and the committed slot gets its own" "$CALLS" "efiboot forget efiboot ensure a "
+check "... before its record goes" "$(cat "$KTEST/forget-saw" 2>/dev/null)" "kept"
 check "no trial.failed for an interruption" "$([[ -e "$KTEST/boot/trial.failed" ]] && echo present || echo none)" "none"
 run_case legacy a "" persistent 'b\n' $ALL; go
 check "a record without an armed line (older updater) counts as armed" "$RESULT" "trial-failed b"

tools/update/kryptik-recover

@@ -59,6 +59,8 @@ slot_dev() { case "$1" in a) echo "$SA" ;; b) echo "$SB" ;; *) die "slot must be
 has_fs() { [ "$(dd if="$1" bs=1 skip=1080 count=2 status=none | od -An -tx1 | tr -d ' \n')" = "53ef" ]; }
 # Written, fsynced, renamed, as the kernels are: a power cut leaves the old file whole, not empty.
 put() { printf '%s\n' "$2" > "$1.new" && sync -f "$1.new" && mv -f "$1.new" "$1"; }   # put FILE LINE
+# Drop FILE's clean pages, so the next read is what the device holds; best effort.
+uncache() { dd if="$1" iflag=nocache count=0 status=none 2>/dev/null || :; }   # uncache FILE
 
 if [ "$STATUS" = 1 ]; then
     mount -t vfat -o ro "$ESP" /run/kryptik-recover || die "cannot mount the ESP"
@@ -91,6 +93,8 @@ commit_slot() {   # commit_slot SLOT (ESP mounted rw at /run/kryptik-recover)
     has_fs "$(slot_dev "$s")" || die "slot $s holds no filesystem; use --restore-slot $s"
     cp "$k" /run/kryptik-recover/EFI/BOOT/BOOTX64.EFI.new
     sync -f /run/kryptik-recover/EFI/BOOT/BOOTX64.EFI.new
+    uncache /run/kryptik-recover/EFI/BOOT/BOOTX64.EFI.new
+    cmp -s "$k" /run/kryptik-recover/EFI/BOOT/BOOTX64.EFI.new || die "slot $s's kernel did not copy whole; BOOTX64.EFI is unchanged"
     mv -f /run/kryptik-recover/EFI/BOOT/BOOTX64.EFI.new /run/kryptik-recover/EFI/BOOT/BOOTX64.EFI
     put /run/kryptik-recover/kryptik/committed-slot "$s" || die "cannot record slot $s as committed"
     sync
@@ -148,6 +152,9 @@ if [ -n "$RESTORE" ]; then
     mount -t vfat -o rw "$ESP" /run/kryptik-recover || die "cannot mount the target ESP"
     cp "$kdir/kryptik-$RESTORE.efi" "/run/kryptik-recover/EFI/kryptik/kryptik-$RESTORE.efi.new"
     sync -f "/run/kryptik-recover/EFI/kryptik/kryptik-$RESTORE.efi.new"
+    uncache "/run/kryptik-recover/EFI/kryptik/kryptik-$RESTORE.efi.new"
+    cmp -s "$kdir/kryptik-$RESTORE.efi" "/run/kryptik-recover/EFI/kryptik/kryptik-$RESTORE.efi.new" \
+        || die "the medium's kernel for slot $RESTORE did not copy whole; nothing was committed"
     mv -f "/run/kryptik-recover/EFI/kryptik/kryptik-$RESTORE.efi.new" "/run/kryptik-recover/EFI/kryptik/kryptik-$RESTORE.efi"
     put "/run/kryptik-recover/kryptik/version-$RESTORE" "$ver" || die "cannot record slot ${RESTORE}'s version"
     commit_slot "$RESTORE"

tools/update/kryptik-update

@@ -76,6 +76,9 @@ esp_dev() { own_part kryptik-esp; }
 slot_dev() { own_part "kryptik-$1"; }
 hdr() { awk -F': ' -v k="$2" '$1==k {print $2; exit}' "$1"; }
 
+# Drop FILE's clean pages, so the next read is what the device holds; best effort.
+uncache() { dd if="$1" iflag=nocache count=0 status=none 2>/dev/null || :; }   # uncache FILE
+
 mount_esp() {
     dev="$(esp_dev)" || exit 1
     mkdir -p "$ESP_MNT"
@@ -364,7 +367,9 @@ cmd_apply() {
     mkdir -p "$ESP_MNT/EFI/kryptik" "$ESP_MNT/kryptik"
     cp "$(payload_path "kryptik-$target.efi")" "$ESP_MNT/EFI/kryptik/kryptik-$target.efi.new" || die "staging the kernel on the ESP failed"
     sync -f "$ESP_MNT/EFI/kryptik/kryptik-$target.efi.new" || die "the kernel staged on the ESP could not be flushed"
-    # Check the staged kernel before it gets the name rollback and recovery use.
+    # Check the staged kernel, as the device holds it, before it gets the name
+    # rollback and recovery use.
+    uncache "$ESP_MNT/EFI/kryptik/kryptik-$target.efi.new"
     want_k="$H_KA"; [ "$target" = b ] && want_k="$H_KB"
     if [ "$(sha256sum "$ESP_MNT/EFI/kryptik/kryptik-$target.efi.new" | cut -c1-64)" != "$want_k" ]; then
         rm -f "$ESP_MNT/EFI/kryptik/kryptik-$target.efi.new"