A commit whose committed-slot record cannot be written is a failed commit: the trial stays, and the next boot commits again

build/service-scripts/boot-success.sh

@@ -83,9 +83,13 @@ commit_slot() {   # commit_slot <slot>: make BOOTX64.EFI this slot's kernel
             cp "$src" "$dst.new" && sync -f "$dst.new" && mv -f "$dst.new" "$dst" && sync -f "$dst" && rc=0
             [ "$rc" -eq 0 ] && say "committed: BOOTX64.EFI is now slot $1"
         fi
-        cp "$ESP_MNT/kryptik/version-$1" "$ESP_MNT/kryptik/version-committed" 2>/dev/null || true
-        printf '%s\n' "$1" > "$ESP_MNT/kryptik/committed-slot.new" && \
-            mv -f "$ESP_MNT/kryptik/committed-slot.new" "$ESP_MNT/kryptik/committed-slot"
+        # The record is part of the commit, and follows the boot file: kryptik-update
+        # applies only from the slot it names, so it must not name one that does not boot.
+        if [ "$rc" -eq 0 ]; then
+            { printf '%s\n' "$1" > "$ESP_MNT/kryptik/committed-slot.new" && sync -f "$ESP_MNT/kryptik/committed-slot.new" && \
+                mv -f "$ESP_MNT/kryptik/committed-slot.new" "$ESP_MNT/kryptik/committed-slot"; } \
+                || { say "the committed-slot record could not be written; the trial stays, and the next boot commits again"; rc=1; }
+        fi
     else
         say "no kernel for slot $1 on the ESP"
     fi
@@ -150,7 +154,7 @@ if [ -n "$trial" ]; then
                 say "$(kryptikd time committed "$B/release-$slot" 2>&1)"
             else
                 result "commit-failed $slot"
-                say "slot $slot is healthy but the commit failed; the committed slot is unchanged"
+                say "slot $slot is healthy but the commit failed; the trial stays for the next boot"
             fi
         else
             say "trial slot $slot came up UNHEALTHY:"

docs/design/boot-and-updates.md

@@ -143,14 +143,14 @@ payloads come from [the update channel](update-channel.md) or by hand.
    cut short leaves nothing that `rollback` or `kryptik-recover --commit-slot`
    would take. Write the slot (`dd conv=fsync`) and read it back.
 3. Put its kernel on the ESP as `.efi.new`, fsync, check it, rename; write
-   its version file. Keep the verified manifest and signature in
+   its version file the same way. Keep the verified manifest and signature in
    `/var/lib/kryptik/boot/release-<slot>/` for the [clock's floor](time.md).
 4. Record the trial (`armed=0`), run `kryptik-efiboot set-next <inactive>`,
    record `armed=1`, reboot.
 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`,
+   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
@@ -181,7 +181,7 @@ payloads come from [the update channel](update-channel.md) or by hand.
    who rewrites the ESP itself: the committed slot's name there is not
    signed.
 
-Zone data is never written. On the FAT ESP the two renames are the only
+Zone data is never written. On the FAT ESP the renames are the only
 non-atomic steps; each follows a complete, fsynced copy and leaves a system
 that boots either way. From the medium, `kryptik-recover` commits or rewrites
 a slot and restores the state header ([user guide](../user-guide.md)).

tools/tests/boot-success.sh

@@ -66,7 +66,11 @@ echo "umount $1" >> "$KTEST/calls"
 EOF
 cat > "$T/bin/sync" <<'EOF'
 #!/bin/sh
-exit 0
+echo "sync $*" >> "$KTEST/syncs"
+case "$*" in
+    *committed-slot.new) [ ! -e "$KTEST/sync_fails" ] ;;
+    *BOOTX64.EFI.new) [ ! -e "$KTEST/sync_fails_boot" ] ;;
+esac
 EOF
 chmod +x "$T"/bin/*
 
@@ -126,8 +130,15 @@ run_case commit b "" persistent 'b\narmed=1\n' $ALL; go
 check "healthy trial: committed" "$RESULT" "commit b"
 check "BOOTX64.EFI is now the slot b kernel" "$(cat "$KTEST/esp/EFI/BOOT/BOOTX64.EFI")" "kernel-b"
 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 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 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"
 

tools/update/kryptik-update

@@ -363,16 +363,18 @@ cmd_apply() {
     mount_esp
     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"
+    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.
     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"
         die "the kernel staged on the ESP does not hash to the manifest's kryptik-$target.efi"
     fi
     mv -f "$ESP_MNT/EFI/kryptik/kryptik-$target.efi.new" "$ESP_MNT/EFI/kryptik/kryptik-$target.efi"
-    printf '%s\n' "$VERSION" > "$ESP_MNT/kryptik/version-$target.new"
-    mv -f "$ESP_MNT/kryptik/version-$target.new" "$ESP_MNT/kryptik/version-$target"
+    { printf '%s\n' "$VERSION" > "$ESP_MNT/kryptik/version-$target.new" \
+        && sync -f "$ESP_MNT/kryptik/version-$target.new" \
+        && mv -f "$ESP_MNT/kryptik/version-$target.new" "$ESP_MNT/kryptik/version-$target"; } \
+        || die "slot $target's version could not be recorded on the ESP"
     sync
     umount_esp
     say "kernel for slot $target installed on the ESP (version $VERSION)"