block: Fix QCOW double allocation after a failed L2 relocation

The relocate-on-write path in update_cluster_addr() pushes the old L2
table cluster onto unref_clusters before allocating the relocation
target. When that allocation fails at allocator exhaustion, the error
unwinds with the still-referenced table on the free list, its deferred
refcount update dropped, the L1 entry unchanged and the cached table
still clean. The next metadata flush publishes the cluster to the
allocator, and a later allocation hands the live L2 table to a new
writer, which overwrites it with a refcount block, another L2 table or
guest data. Retried writes against the same L1 slot re-enter the
clean-table branch and push further duplicates, so one cluster can also
be handed out to two writers at once.

Images holding compressed clusters reach this constantly: writing to a
compressed cluster always takes the decompress -> append_data_cluster
-> update_cluster_addr path, so every such write relocates its L2
table.

Allocate the relocation target first and release the old table only
once the allocation has succeeded. A failed relocation now leaves the
old table referenced, off the free lists and intact.

Add deterministic regression coverage for both entry points: a plain
write that exhausts the allocator at the L2 relocation step, and a
compressed-cluster write that triggers the same relocation.

Signed-off-by: doge <me@crackerben.com>

block/src/formats/qcow/metadata.rs

@@ -728,6 +728,15 @@ impl QcowState {
         set_refcounts: &mut Vec<(u64, u64)>,
     ) -> io::Result<()> {
         if !self.l2_cache.get(l1_index).unwrap().dirty() {
+            // Allocate the new cluster for the relocated L2 table before
+            // releasing the old one: if this allocation fails (ENOSPC at
+            // allocator exhaustion) the old table must stay off the free
+            // lists, or a later allocation would hand it out and overwrite a
+            // live L2 table (issue #8606). The cluster will be written when
+            // the cache is flushed.
+            let new_addr = self.get_new_cluster(None)?;
+            set_refcounts.push((new_addr, 1));
+
             // Free the previously used cluster if one exists. Modified tables are always
             // written to new clusters so the L1 table can be committed to disk after they
             // are and L1 never points at an invalid table.
@@ -737,10 +746,6 @@ impl QcowState {
                 set_refcounts.push((addr, 0));
             }
 
-            // Allocate a new cluster to store the L2 table and update the L1 table to point
-            // to the new table. The cluster will be written when the cache is flushed.
-            let new_addr = self.get_new_cluster(None)?;
-            set_refcounts.push((new_addr, 1));
             self.l1_table[l1_index] = new_addr; // marks l1_table dirty via IndexMut
         }
         // Write the L2 entry - IndexMut marks the L2 table dirty automatically.
@@ -1136,3 +1141,163 @@ impl QcowState {
         }
     }
 }
+
+#[cfg(test)]
+mod unit_tests {
+    // Regression for the ENOSPC unwind on the relocate-on-write path: when
+    // the allocation for the relocated table fails, the still-referenced old
+    // L2 table must not be left on the free lists (issue #8606).
+    #[test]
+    fn failed_l2_relocate_keeps_live_table_off_free_lists() {
+        let cluster_size: u64 = 1 << 16;
+        let temp = super::super::QcowTempDisk::new(64 * cluster_size, None, false, true, false)
+            .unwrap()
+            .into_tempfile();
+        let raw = crate::AlignedFile::new(temp.as_file().try_clone().unwrap(), false);
+        let (mut inner, _backing, _sparse) =
+            super::super::parser::parse_qcow(raw, 0, true).unwrap();
+
+        // Materialize an L2 table plus one data cluster, then flush so the
+        // next write to the region takes the relocate-on-write path.
+        let super::ClusterWriteMapping::Allocated {
+            offset: data_cluster,
+        } = inner.map_write(0, None).expect("initial write");
+        inner.sync_caches().expect("flush");
+        let live_l2 = inner.l1_table[0];
+        assert_ne!(live_l2, 0);
+
+        // Exhaust the allocator down to two reusable clusters: shrink the
+        // refcount horizon so the file cannot grow, and leave exactly enough
+        // for the data cluster of the next write plus the refcount-block
+        // relocation it triggers, but nothing for relocating its L2 table.
+        let file_clusters = inner
+            .raw_file
+            .file_mut()
+            .metadata()
+            .unwrap()
+            .len()
+            .div_ceil(cluster_size);
+        inner.refcounts = super::super::refcount::RefCount::new(
+            &mut inner.raw_file,
+            inner.header.refcount_table_offset,
+            1,
+            file_clusters,
+            cluster_size,
+            16,
+        )
+        .unwrap();
+        let freed = super::mem::take(&mut inner.unref_clusters);
+        assert!(
+            !freed.is_empty(),
+            "the first write must have relocated the refcount block, freeing its old cluster"
+        );
+        inner.avail_clusters.clear();
+        inner.avail_clusters.push(data_cluster);
+        inner.avail_clusters.extend(freed);
+
+        let err = inner
+            .map_write(cluster_size, None)
+            .expect_err("relocation must fail with the allocator exhausted");
+        assert_eq!(err.raw_os_error(), Some(libc::ENOSPC));
+        assert!(
+            inner.avail_clusters.is_empty(),
+            "the failure must land on the L2 relocation, with every candidate consumed"
+        );
+
+        assert_eq!(
+            inner.l1_table[0], live_l2,
+            "L1 must still reference the old table after the failed write"
+        );
+        assert!(
+            !inner.unref_clusters.contains(&live_l2) && !inner.avail_clusters.contains(&live_l2),
+            "a still-referenced L2 table must never enter the free lists"
+        );
+    }
+
+    // The same defect is reached constantly by images holding compressed
+    // clusters: writing to a compressed cluster always takes the
+    // decompress -> append_data_cluster -> update_cluster_addr path, so every
+    // such write relocates its L2 table.
+    #[test]
+    fn failed_l2_relocate_after_compressed_write_keeps_live_table() {
+        use std::os::unix::fs::FileExt;
+
+        const COMPRESSED_FLAG: u64 = 1 << 62;
+        let cluster_size: u64 = 1 << 16;
+        let temp = super::super::QcowTempDisk::new(64 * cluster_size, None, false, true, false)
+            .unwrap()
+            .into_tempfile();
+
+        // Seed an L2 table plus a data cluster, flush, then compress that data
+        // cluster in place so re-writing it takes the compressed path.
+        {
+            let raw = crate::AlignedFile::new(temp.as_file().try_clone().unwrap(), false);
+            let (mut inner, _backing, _sparse) =
+                super::super::parser::parse_qcow(raw, 0, true).unwrap();
+            inner
+                .map_write(0, Some(vec![0xab; cluster_size as usize]))
+                .expect("seed write");
+            inner.sync_caches().expect("flush");
+        }
+        super::super::common::unit_tests::compress_allocated_clusters(
+            &mut temp.as_file().try_clone().unwrap(),
+        );
+
+        // Re-open so the L2 cache reflects the on-disk compressed entry.
+        let raw = crate::AlignedFile::new(temp.as_file().try_clone().unwrap(), false);
+        let (mut inner, _backing, _sparse) =
+            super::super::parser::parse_qcow(raw, 0, true).unwrap();
+        let live_l2 = inner.l1_table[0];
+        assert_ne!(live_l2, 0);
+
+        // An already allocated plain cluster would be written in place with no
+        // relocation, so confirm the seed really is compressed.
+        let mut entry = [0u8; 8];
+        temp.as_file().read_exact_at(&mut entry, live_l2).unwrap();
+        assert_ne!(
+            u64::from_be_bytes(entry) & COMPRESSED_FLAG,
+            0,
+            "the seed cluster must be compressed to trigger the relocation"
+        );
+
+        // Cap file growth at the current horizon and leave exactly two free
+        // clusters: enough for the write's data cluster and the refcount block
+        // relocation it triggers, but nothing for relocating the L2 table.
+        let file_clusters = inner
+            .raw_file
+            .file_mut()
+            .metadata()
+            .unwrap()
+            .len()
+            .div_ceil(cluster_size);
+        inner.refcounts = super::super::refcount::RefCount::new(
+            &mut inner.raw_file,
+            inner.header.refcount_table_offset,
+            1,
+            file_clusters,
+            cluster_size,
+            16,
+        )
+        .unwrap();
+        let mut free = super::mem::take(&mut inner.avail_clusters);
+        free.retain(|&c| c != live_l2 && c != 0);
+        assert!(free.len() >= 2, "need two free clusters for the budget");
+        inner.avail_clusters.clear();
+        inner.avail_clusters.push(free[0]);
+        inner.avail_clusters.push(free[1]);
+
+        let err = inner
+            .map_write(0, None)
+            .expect_err("the L2 relocation must fail with the allocator exhausted");
+        assert_eq!(err.raw_os_error(), Some(libc::ENOSPC));
+
+        assert_eq!(
+            inner.l1_table[0], live_l2,
+            "L1 must still reference the old table after the failed write"
+        );
+        assert!(
+            !inner.unref_clusters.contains(&live_l2) && !inner.avail_clusters.contains(&live_l2),
+            "a still-referenced L2 table must never enter the free lists"
+        );
+    }
+}