From a6578ace1cc41f4987ac2488abd94a729aa1d507 Mon Sep 17 00:00:00 2001 From: Kun Lai Date: Thu, 8 Oct 2026 16:29:57 +0800 Subject: [PATCH 1/4] fix(fde): initialize a full snapshot chunk when wiping the COW header dm-snap-persistent reads and writes whole chunks, so wiping only the first 4 KiB of the COW device left sector 8 (the first sector beyond the wipe) without a valid integrity tag. The snapshot's header read then failed with "INTEGRITY AEAD ERROR, sector 8" and the dm-snapshot creation failed on the first boot of a disk-persist + integrity=true delta. The chunk size is now a constant shared by the wipe and the snapshot table, and the wipe writes a full chunk so the whole header region carries valid tags. blkid's "no signatures" is not proof of a blank device: its end-of-device probe read fails on a delta whose integrity tags were never written, and libblkid turns that failure into "clean". On an existing volume that report caused a wipe of the previous boot's COW metadata. The persistent-mode wipe now reads the header chunk itself: unreadable aborts (fail closed), non-zero content is kept for the snapshot to interpret (valid SnAp metadata gets reused, anything else makes dmsetup fail closed), all zeros mean blank. On a freshly created delta the COW is blank by construction, so the probe is skipped and the header chunk is initialized directly. Fixes #141. Assisted-by: Claude:glm-5.3 Signed-off-by: Kun Lai --- .../cmd/boot_service/stage/before_sysroot.rs | 152 ++++++++++++++++-- 1 file changed, 139 insertions(+), 13 deletions(-) diff --git a/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs b/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs index 801c5aba..c2f20954 100644 --- a/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs +++ b/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs @@ -24,6 +24,15 @@ use cryptpilot::{ const CRYPTPILOT_LVM_SYSTEM_DIR: &str = "/usr/lib/cryptpilot/lvm/"; +/// dm-snapshot chunk size in 512-byte sectors, shared by the snapshot table +/// and the COW header initialization: the persistent snapshot driver reads +/// and writes whole chunks, so any region the guest initializes on the COW +/// device must cover a full chunk or the read fails (with dm-integrity, +/// sectors never written through the mapping have no valid tag). +const SNAPSHOT_CHUNK_SIZE_SECTORS: u64 = 16; + +const SNAPSHOT_CHUNK_SIZE_BYTES: usize = SNAPSHOT_CHUNK_SIZE_SECTORS as usize * 512; + pub async fn setup_volumes_required_by_fde() -> Result<()> { let fde_config = crate::config::get_fde_config_source() .await @@ -165,6 +174,7 @@ pub async fn setup_volumes_required_by_fde() -> Result<()> { dm_verity_output_device, Path::new(DELTA_DEVICE), matches!(delta_location, DeltaLocation::DiskPersist), + recreate, ) .await?; @@ -191,8 +201,13 @@ pub async fn setup_volumes_required_by_fde() -> Result<()> { tracing::info!("Creating zram device for COW storage"); let cow_device = create_zram_cow_device().await?; // Build dm-snapshot device chain - setup_dm_snapshot_device_chain(dm_verity_output_device, &cow_device, false) - .await?; + setup_dm_snapshot_device_chain( + dm_verity_output_device, + &cow_device, + false, + false, + ) + .await?; // Resize rootfs filesystem to fill the expanded device after building snapshot chain resize_ext4_filesystem(Path::new(ROOTFS_DEVICE)).await?; } @@ -437,8 +452,11 @@ async fn create_zram_cow_device() -> Result { Ok(PathBuf::from(format!("/dev/zram{}", zram_id))) } -/// Wipe a device by writing zeros using tokio async I/O. -/// Writes 4KB megabytes of zeros to the device. +/// Zero the COW header region through the device mapping. +/// +/// Must cover a full snapshot chunk: the persistent snapshot driver reads +/// and writes whole chunks, and with dm-integrity only sectors written +/// through the mapping carry a valid tag. async fn wipe_cow_device_header_async(device_path: &Path) -> Result<(), anyhow::Error> { use tokio::fs::OpenOptions; @@ -448,18 +466,53 @@ async fn wipe_cow_device_header_async(device_path: &Path) -> Result<(), anyhow:: .await .context("Failed to open device for wiping")?; - file.write_all(&[0u8; 4096]).await?; - - // Final sync to ensure everything is written + file.write_all(&vec![0u8; SNAPSHOT_CHUNK_SIZE_BYTES]) + .await?; file.sync_all().await?; - Ok(()) } +/// Wipe the COW header chunk only when the header region reads back blank. +/// +/// blkid reports "no signatures" both for a genuinely blank device and for +/// one whose probe reads fail (dm-integrity rejects sectors whose tags were +/// never written, e.g. the end-of-device probe), so "no signatures" alone +/// is not proof of blank. The header chunk is: unreadable means we cannot +/// tell (fail closed), non-zero content is left untouched for the snapshot +/// to interpret (valid "SnAp" metadata gets reused; anything else makes +/// dmsetup fail closed), all zeros mean blank and safe to initialize. +async fn wipe_cow_header_after_read_check(device_path: &Path) -> Result<(), anyhow::Error> { + use tokio::fs::OpenOptions; + use tokio::io::AsyncReadExt; + + let mut file = OpenOptions::new() + .read(true) + .open(device_path) + .await + .context("Failed to open COW device for header check")?; + + let mut header = vec![0u8; SNAPSHOT_CHUNK_SIZE_BYTES]; + file.read_exact(&mut header).await.context( + "COW header chunk is unreadable, cannot tell whether the volume is blank; refusing to wipe", + )?; + drop(file); + + if header.iter().any(|&b| b != 0) { + tracing::info!( + "COW header has existing content, keeping it for the snapshot to reuse \ + (blkid misreported the device as clean)" + ); + return Ok(()); + } + + wipe_cow_device_header_async(device_path).await +} + async fn setup_dm_snapshot_device_chain( rootfs_device: &Path, cow_device: &Path, persistent: bool, + fresh_delta: bool, ) -> Result<()> { tracing::info!( ?rootfs_device, @@ -506,6 +559,15 @@ async fn setup_dm_snapshot_device_chain( wipe_cow_device_header_async(cow_device) .await .context("Failed to wipe COW device")?; + } else if fresh_delta { + // The COW device was created on this boot and is blank by + // construction; unwritten sectors fail integrity reads, so probing + // here would report "no signatures" for the wrong reason. Initialize + // the header chunk directly. + tracing::info!("Persistent mode: fresh delta, initializing COW header"); + wipe_cow_device_header_async(cow_device) + .await + .context("Failed to wipe COW device")?; } else { // Persistent mode: probe COW device to determine safe action tracing::info!("Persistent mode: probing COW device state"); @@ -513,9 +575,11 @@ async fn setup_dm_snapshot_device_chain( match probe { cryptpilot::fs::blkid::BlkidProbeResult::NoSignatures => { - // Clean device, safe to wipe - tracing::info!("COW device is clean, wiping"); - wipe_cow_device_header_async(cow_device) + // blkid's "no signatures" also covers probe reads that + // failed, so the header chunk itself decides: wiped only + // when blank, kept when it holds snapshot metadata. + tracing::info!("COW device reports clean, verifying header"); + wipe_cow_header_after_read_check(cow_device) .await .context("Failed to wipe COW device")?; } @@ -546,11 +610,12 @@ async fn setup_dm_snapshot_device_chain( .arg(ROOTFS_NAME) .arg("--table") .arg(format!( - "0 {} snapshot {} {} {} 16", // chunk size is 16 sectors (8KB) + "0 {} snapshot {} {} {} {}", linear_size, ROOTFS_EXTENDED_DEVICE, cow_device.to_string_lossy(), - if persistent { "PO" } else { "N" } + if persistent { "PO" } else { "N" }, + SNAPSHOT_CHUNK_SIZE_SECTORS )) .run() .await @@ -649,6 +714,67 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_wipe_cow_header_covers_full_snapshot_chunk() -> Result<()> { + let dir = tempfile::tempdir()?; + let cow = dir.path().join("cow.img"); + // Non-zero pattern makes a partial wipe visible. + tokio::fs::write(&cow, vec![0xAAu8; SNAPSHOT_CHUNK_SIZE_BYTES * 2]).await?; + + wipe_cow_device_header_async(&cow).await?; + + let data = tokio::fs::read(&cow).await?; + assert!( + data[..SNAPSHOT_CHUNK_SIZE_BYTES].iter().all(|&b| b == 0), + "wipe must zero the whole first chunk the snapshot driver reads" + ); + assert!(data[SNAPSHOT_CHUNK_SIZE_BYTES..].iter().all(|&b| b == 0xAA)); + Ok(()) + } + + #[tokio::test] + async fn test_guarded_wipe_requires_readable_header_chunk() -> Result<()> { + let dir = tempfile::tempdir()?; + let cow = dir.path().join("short.img"); + // Shorter than one chunk: reading the header region fails, so the + // wipe must refuse rather than treat the failure as a blank device. + tokio::fs::write(&cow, vec![0xAAu8; SNAPSHOT_CHUNK_SIZE_BYTES / 2]).await?; + + let result = wipe_cow_header_after_read_check(&cow).await; + + assert!(result.is_err(), "unreadable header must abort the wipe"); + let data = tokio::fs::read(&cow).await?; + assert!( + data.iter().all(|&b| b == 0xAA), + "refused wipe must leave the device untouched" + ); + Ok(()) + } + + #[tokio::test] + async fn test_guarded_wipe_preserves_nonzero_header() -> Result<()> { + let dir = tempfile::tempdir()?; + let cow = dir.path().join("snapcow.img"); + // dm-snapshot persistent metadata starts with the "SnAp" magic. + // blkid misreports such a device as clean when its probe reads fail + // under dm-integrity, so the wipe must verify the header content + // itself and keep anything non-zero for the snapshot to interpret. + let mut header = vec![0u8; SNAPSHOT_CHUNK_SIZE_BYTES]; + header[0..4].copy_from_slice(b"SnAp"); + header[8..12].copy_from_slice(&1u32.to_le_bytes()); + header[12..16].copy_from_slice(&(SNAPSHOT_CHUNK_SIZE_SECTORS as u32).to_le_bytes()); + tokio::fs::write(&cow, &header).await?; + + wipe_cow_header_after_read_check(&cow).await?; + + let data = tokio::fs::read(&cow).await?; + assert_eq!( + data, header, + "non-zero header holds dm-snapshot metadata; wiping it would destroy the previous boot's delta" + ); + Ok(()) + } + #[tokio::test] async fn test_resize_clean_ext4_checked_before_last_mount() -> Result<()> { let (_dir, image) = ext4_image().await?; From 31b9678d9269bdd836f06321cce61f72478b53cd Mon Sep 17 00:00:00 2001 From: Kun Lai Date: Thu, 8 Oct 2026 16:30:06 +0800 Subject: [PATCH 2/4] ci(convert): boot the disk-persist integrity scenario twice The fsck-141 scenario now re-boots the same persistent overlay after the first boot reaches login, covering the issue #141 request for a first-boot plus same-disk second-boot test: the second boot must keep and reuse the persisted COW metadata. Also stop treating a bare "INTEGRITY AEAD ERROR" console line as a boot failure. blkid probing the end of a freshly created delta always triggers those errors (sectors whose tags were never written), so the marker made the test fail on a healthy boot. The fatal markers are the dmsetup failure, emergency mode, or the FDE service failing. Assisted-by: Claude:glm-5.3 Signed-off-by: Kun Lai --- .github/workflows/build-rpm.yml | 15 +++++++++------ tests/test-convert.sh | 7 ++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build-rpm.yml b/.github/workflows/build-rpm.yml index 61af6950..965b0797 100644 --- a/.github/workflows/build-rpm.yml +++ b/.github/workflows/build-rpm.yml @@ -335,12 +335,15 @@ jobs: # Regression coverage for issues #140 / #141 on disk-persist + stable-key # (the combinations the 24-job test-convert matrix does NOT exercise, since # it uses the otp key provider which recreates the delta every boot). + # Both scenarios boot, then RE-boot the same persistent overlay; both + # boots must reach login. # fsck-141: disk-persist + integrity=true -> first-boot dm-snapshot - # creation must succeed (COW header fully initialized through - # the authenticated mapping). - # fsck-140: disk-persist + integrity=false -> boot, then RE-boot the same - # persistent overlay; both boots must reach login (offline - # resize2fs must not refuse a clean fs with s_lastcheck offline resize2fs must not + # refuse a clean fs with s_lastcheck < s_mtime on the second + # boot (e2fsck runs first). # UKI only (both bugs fire in the dracut FDE service, bootloader-agnostic). # +4 jobs: 2 distros x 2 scenarios. test-convert-persist: @@ -363,7 +366,7 @@ jobs: - scenario: fsck-141 delta_key: stable integrity: "true" - second_boot: "" + second_boot: "true" runs-on: ubuntu-latest needs: build env: diff --git a/tests/test-convert.sh b/tests/test-convert.sh index c1440423..ba743ceb 100755 --- a/tests/test-convert.sh +++ b/tests/test-convert.sh @@ -765,7 +765,12 @@ exec qemu-system-x86_64 \ boot_success=true break fi - if grep -qi "Emergency Mode\|emergency shell\|Kernel panic\|Failed to setup volumes required by FDE\|Failed to create dm-snapshot\|INTEGRITY AEAD ERROR\|Please run 'e2fsck\|Failed to resize ext4\|Failed to start Cryptpilot FDE" "${boot_log}" 2>/dev/null; then + # NOTE: bare "INTEGRITY AEAD ERROR" is not a boot-failure marker: + # blkid probing the end of a freshly created delta reads sectors + # whose integrity tags were never initialized, and those errors are + # noise. The fatal markers are the dmsetup failure, emergency mode, + # or the FDE service failing. + if grep -qi "Emergency Mode\|emergency shell\|Kernel panic\|Failed to setup volumes required by FDE\|Failed to create dm-snapshot device\|reload ioctl .* failed\|Please run 'e2fsck\|Failed to resize ext4\|Failed to start Cryptpilot FDE" "${boot_log}" 2>/dev/null; then log::error "Boot failure detected - boot failed!" break fi From 7b031b5490c821285789ca7b4e582b7a1b8406fb Mon Sep 17 00:00:00 2001 From: Kun Lai Date: Thu, 8 Oct 2026 17:35:44 +0800 Subject: [PATCH 3/4] ci(convert): start dockerd for the direct QEMU boot on alinux4 The direct qemu boot path skipped the container-runtime setup that the qemus-entrypoint path performs. On alinux4 docker is the real Docker Engine and the test container has no init system, so dockerd was never running and the QEMU container exited instantly, failing both persist scenarios within seconds. Run the same ensure_docker_runtime step and, when dockerd was started by us (no bridge configured), run the QEMU container with the host network namespace so guest user-mode networking works. Assisted-by: Claude:glm-5.3 Signed-off-by: Kun Lai --- tests/test-convert.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test-convert.sh b/tests/test-convert.sh index ba743ceb..04be3f5d 100755 --- a/tests/test-convert.sh +++ b/tests/test-convert.sh @@ -739,8 +739,22 @@ test_qemu_boot_direct() { log::step "Direct QEMU boot for: ${test_name} (cpu=${cpu_cores}, ram=${ram_size}, image=${image_bn})" + # Alinux 4 ships real Docker (not podman-docker) and the test container + # has no init system, so dockerd must be started explicitly. No-op on + # Alinux 3 where docker is the daemonless podman emulation. A self-started + # dockerd has no bridge, so the QEMU container needs the host network + # namespace for guest user-mode networking (NTP time sync). + if ! ensure_docker_runtime; then + return 1 + fi + local docker_net_args=() + if [[ "${DOCKERD_STARTED:-0}" == "1" ]]; then + docker_net_args+=(--network=host) + fi + docker rm -f "${container_name}" 2>/dev/null || true docker run -d --rm --privileged \ + "${docker_net_args[@]}" \ -v "$(dirname "${image}"):/diskdir:rw" \ --name "${container_name}" --entrypoint /bin/bash \ "ghcr.io/qemus/qemu:7.29" -c ' From 6ce9c518ceaf9a58e0e5055dad4b29b1a6813658 Mon Sep 17 00:00:00 2001 From: Kun Lai Date: Thu, 8 Oct 2026 19:11:56 +0800 Subject: [PATCH 4/4] ci(convert): create the persist overlay next to the output image The persist overlay was created in the workdir root while the converted image lives in the per-test subdirectory, so the overlay's bare relative backing filename never resolved and qemu exited instantly on the missing file (the failure was swallowed because the function runs inside an if condition, leaving only an unhelpful "container exited unexpectedly"). Create the overlay in the image's directory, where the boot container's single mount covers both files, and fail loudly when overlay creation fails. Assisted-by: Claude:glm-5.3 Signed-off-by: Kun Lai --- tests/test-convert.sh | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/test-convert.sh b/tests/test-convert.sh index 04be3f5d..78d6953d 100755 --- a/tests/test-convert.sh +++ b/tests/test-convert.sh @@ -818,13 +818,18 @@ test_qemu_boot_persist_second() { local cpu_cores="${3:-4}" local ram_size="${4:-4G}" - local overlay="${WORKDIR}/${test_name}-persist-overlay.qcow2" - local overlay_bn; overlay_bn=$(basename "${overlay}") + # The overlay must live in the SAME directory as the output image: the + # backing file is referenced by its bare name so qemu resolves it inside + # the boot container (host workdir <-> container /diskdir), and the boot + # container only mounts that one directory. + local image_dir; image_dir="$(dirname "${output_image}")" + local overlay="${image_dir}/${test_name}-persist-overlay.qcow2" rm -f "${overlay}" - # Relative backing filename so the path resolves inside the container - # (host workdir <-> container /diskdir share the same directory). - ( cd "$(dirname "${overlay}")" \ - && qemu-img create -f qcow2 -F qcow2 -b "$(basename "${output_image}")" "${overlay_bn}" >/dev/null ) + if ! ( cd "${image_dir}" \ + && qemu-img create -f qcow2 -F qcow2 -b "$(basename "${output_image}")" "$(basename "${overlay}")" >/dev/null ); then + log::error "Failed to create persistent overlay in ${image_dir} for: ${test_name}" + return 1 + fi log::step "Persist two-boot test for: ${test_name}" log::info "Boot #1 (expect success; fs mounted RW updates s_mtime via the persistent delta)"