Most code in qcow2 that accesses (and potentially modifies) L2 tables
does so while holding s->lock.

There is one exception, which is allocating writes. They hold the lock
initially while allocating clusters, but drop it for writing the guest
payload before taking the lock again for updating the L2 tables. This
allows concurrent requests that touch other parts of the image file to
continue in parallel and is an important performance optimisation.

However, this means that other requests that run while the lock is
dropped for writing guest data must synchronise with the list of
allocating requests in s->cluster_allocs and wait if they would overlap.
For writes, this is done in handle_dependencies(), but discard and write
zeros operations neglect to synchronise with s->cluster_allocs.

This means that discard can free a cluster whose L2 entry will already
be modified in qcow2_alloc_cluster_link_l2() by a previously started
write. In the case of a pre-allocated zero cluster that is in the
process of being overwritten, this means that discard can lead to a
situation where the cluster is still mapped (because the write will
restore the L2 entry just without the zero flag), but its refcount has
been decreased, resulting in a corrupted image.

Add the missing synchronisation to qcow2_cluster_discard() and
qcow2_subcluster_zeroize() to fix the problem.

Cc: qemu-stable@nongnu.org
Reported-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c | 52 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index c655bf6df42..8b1e80bd0b3 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1392,6 +1392,9 @@ count_single_write_clusters(BlockDriverState *bs, int nb_clusters,
  * the same cluster. In this case we need to wait until the previous
  * request has completed and updated the L2 table accordingly.
  *
+ * If allow_shortening == true, instead of waiting for a dependency, *cur_bytes
+ * can be shortened so that the cluster allocations don't overlap.
+ *
  * Returns:
  *   0       if there was no dependency. *cur_bytes indicates the number of
  *           bytes from guest_offset that can be read before the next
@@ -1403,7 +1406,9 @@ count_single_write_clusters(BlockDriverState *bs, int nb_clusters,
  */
 static int coroutine_fn handle_dependencies(BlockDriverState *bs,
                                             uint64_t guest_offset,
-                                            uint64_t *cur_bytes, QCowL2Meta **m)
+                                            uint64_t *cur_bytes,
+                                            bool allow_shortening,
+                                            QCowL2Meta **m)
 {
     BDRVQcow2State *s = bs->opaque;
     QCowL2Meta *old_alloc;
@@ -1434,7 +1439,7 @@ static int coroutine_fn handle_dependencies(BlockDriverState *bs,
 
         /* Conflict */
 
-        if (start < old_start) {
+        if (start < old_start && allow_shortening) {
             /* Stop at the start of a running allocation */
             bytes = old_start - start;
         } else {
@@ -1469,6 +1474,29 @@ static int coroutine_fn handle_dependencies(BlockDriverState *bs,
     return 0;
 }
 
+static void coroutine_mixed_fn wait_for_dependencies(BlockDriverState *bs,
+                                                     uint64_t guest_offset,
+                                                     uint64_t bytes)
+{
+    BDRVQcow2State *s = bs->opaque;
+    QCowL2Meta *m = NULL;
+    int ret;
+
+    /*
+     * Discard has some non-coroutine callers (creating internal snapshots and
+     * make empty). They are calling from qemu-img or in a drained section, so
+     * we know that no writes can be in progress.
+     */
+    if (!qemu_in_coroutine()) {
+        assert(QLIST_EMPTY(&s->cluster_allocs));
+        return;
+    }
+
+    do {
+        ret = handle_dependencies(bs, guest_offset, &bytes, false, &m);
+    } while (ret == -EAGAIN);
+}
+
 /*
  * Checks how many already allocated clusters that don't require a new
  * allocation there are at the given guest_offset (up to *bytes).
@@ -1840,7 +1868,7 @@ again:
          *         the right synchronisation between the in-flight request and
          *         the new one.
          */
-        ret = handle_dependencies(bs, start, &cur_bytes, m);
+        ret = handle_dependencies(bs, start, &cur_bytes, true, m);
         if (ret == -EAGAIN) {
             /* Currently handle_dependencies() doesn't yield if we already had
              * an allocation. If it did, we would have to clean up the L2Meta
@@ -2000,6 +2028,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
     int64_t cleared;
     int ret;
 
+    /*
+     * If we're touching a cluster for which allocating writes are in flight,
+     * wait for them to complete to avoid conflicting metadata updates.
+     *
+     * We don't need to allocate a QCowL2Meta for the discard operation because
+     * s->lock is held for the duration of the whole operation.
+     */
+    wait_for_dependencies(bs, offset, bytes);
+
     /* Caller must pass aligned values, except at image end */
     assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
     assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
@@ -2160,6 +2197,15 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
     int64_t cleared;
     int ret;
 
+    /*
+     * If we're touching a cluster for which allocating writes are in flight,
+     * wait for them to complete to avoid conflicting metadata updates.
+     *
+     * We don't need to allocate a QCowL2Meta for the zeroize operation because
+     * s->lock is held for the duration of the whole operation.
+     */
+    wait_for_dependencies(bs, offset, bytes);
+
     /* If we have to stay in sync with an external data file, zero out
      * s->data_file first. */
     if (data_file_is_raw(bs)) {
-- 
2.53.0

Dear maintainers,

Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
> Most code in qcow2 that accesses (and potentially modifies) L2 tables
> does so while holding s->lock.
> 
> There is one exception, which is allocating writes. They hold the lock
> initially while allocating clusters, but drop it for writing the guest
> payload before taking the lock again for updating the L2 tables. This
> allows concurrent requests that touch other parts of the image file to
> continue in parallel and is an important performance optimisation.
> 
> However, this means that other requests that run while the lock is
> dropped for writing guest data must synchronise with the list of
> allocating requests in s->cluster_allocs and wait if they would overlap.
> For writes, this is done in handle_dependencies(), but discard and write
> zeros operations neglect to synchronise with s->cluster_allocs.
> 
> This means that discard can free a cluster whose L2 entry will already
> be modified in qcow2_alloc_cluster_link_l2() by a previously started
> write. In the case of a pre-allocated zero cluster that is in the
> process of being overwritten, this means that discard can lead to a
> situation where the cluster is still mapped (because the write will
> restore the L2 entry just without the zero flag), but its refcount has
> been decreased, resulting in a corrupted image.
> 
> Add the missing synchronisation to qcow2_cluster_discard() and
> qcow2_subcluster_zeroize() to fix the problem.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

we had started rolling out a build of QEMU 11 with this patch already
included. However, some of our users reported issues with VMs using
qcow2 disks soon after [0][1]. I was able to reproduce the in-guest
segfaults from [1] in a memory-constrained Debian 12 guest when using a
swap partition on the same disk. Thanks to Thomas for the hunch with
swap! After reverting this patch, I wasn't able to reproduce the issue
anymore. I do not have a better reproducer yet and am not sure about the
exact pattern causing the issue. It's related to the
wait_for_dependencies() call in qcow2_subcluster_zeroize(), because if I
revert just the one in qcow2_cluster_discard(), the issue still reproduces.

Commandline for my reproducer VM [2]. The issue does not happen if I
drop "detect-zeroes":"unmap". Note that I don't have discard-no-unref
for the qcow2 image, so in zero_in_l2_slice(), the branch with
qcow2_free_any_cluster() is taken. Could the conflict be related to that?

I'm still trying to figure things out and come up with a better
reproducer, but wanted to let you know early, also because of the
upcoming stable releases. Of course, I'd also be happy for hints/hunches
and am happy to test suggestions!

Best Regards,
Fiona

[0]: https://forum.proxmox.com/threads/183679/
[1]: https://forum.proxmox.com/threads/183639/

[2]:

> ./qemu-system-x86_64 \
>   -accel kvm \
>   -chardev 'socket,id=qmp,path=/var/run/qemu-server/300.qmp,server=on,wait=off' \
>   -mon 'chardev=qmp,mode=control' \
>   -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect-ms=5000' \
>   -mon 'chardev=qmp-event,mode=control' \
>   -pidfile /var/run/qemu-server/300.pid \
>   -smp '4,sockets=2,cores=2,maxcpus=4' \
>   -nodefaults \
>   -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
>   -vnc 'unix:/var/run/qemu-server/300.vnc,password=on' \
>   -cpu host,+kvm_pv_eoi,+kvm_pv_unhalt \
>   -m 256 \
>   -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
>   -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
>   -device 'pci-bridge,id=pci.3,chassis_nr=3,bus=pci.0,addr=0x5' \
>   -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
>   -device 'virtio-scsi-pci,id=virtioscsi0,bus=pci.3,addr=0x1' \
>   -blockdev '{"detect-zeroes":"unmap","discard":"unmap","driver":"qcow2","file":{"detect-zeroes":"unmap","discard":"unmap","driver":"file","filename":"/mnt/pve/dir/images/300/vm-300-disk-0.qcow2","node-name":"e377549e25f53abd39f9ba01c03653e"},"node-name":"drive-scsi0"}' \
>   -device 'scsi-hd,bus=virtioscsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,device_id=drive-scsi0,bootindex=100' \
>   -netdev 'type=tap,id=net1,ifname=tap300i1,script=/usr/libexec/qemu-server/pve-bridge,downscript=/usr/libexec/qemu-server/pve-bridgedown,vhost=on' \
>   -device 'virtio-net-pci,mac=BC:24:11:CA:B4:EF,netdev=net1,bus=pci.0,addr=0x13,id=net1,rx_queue_size=1024,tx_queue_size=256,host_mtu=1500'

Hi Fiona,

Am 21.05.2026 um 14:12 hat Fiona Ebner geschrieben:
> Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
> > Most code in qcow2 that accesses (and potentially modifies) L2 tables
> > does so while holding s->lock.
> > 
> > There is one exception, which is allocating writes. They hold the lock
> > initially while allocating clusters, but drop it for writing the guest
> > payload before taking the lock again for updating the L2 tables. This
> > allows concurrent requests that touch other parts of the image file to
> > continue in parallel and is an important performance optimisation.
> > 
> > However, this means that other requests that run while the lock is
> > dropped for writing guest data must synchronise with the list of
> > allocating requests in s->cluster_allocs and wait if they would overlap.
> > For writes, this is done in handle_dependencies(), but discard and write
> > zeros operations neglect to synchronise with s->cluster_allocs.
> > 
> > This means that discard can free a cluster whose L2 entry will already
> > be modified in qcow2_alloc_cluster_link_l2() by a previously started
> > write. In the case of a pre-allocated zero cluster that is in the
> > process of being overwritten, this means that discard can lead to a
> > situation where the cluster is still mapped (because the write will
> > restore the L2 entry just without the zero flag), but its refcount has
> > been decreased, resulting in a corrupted image.
> > 
> > Add the missing synchronisation to qcow2_cluster_discard() and
> > qcow2_subcluster_zeroize() to fix the problem.
> > 
> > Cc: qemu-stable@nongnu.org
> > Reported-by: Denis V. Lunev <den@openvz.org>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> 
> we had started rolling out a build of QEMU 11 with this patch already
> included. However, some of our users reported issues with VMs using
> qcow2 disks soon after [0][1]. I was able to reproduce the in-guest
> segfaults from [1] in a memory-constrained Debian 12 guest when using a
> swap partition on the same disk. Thanks to Thomas for the hunch with
> swap! After reverting this patch, I wasn't able to reproduce the issue
> anymore. I do not have a better reproducer yet and am not sure about the
> exact pattern causing the issue. It's related to the
> wait_for_dependencies() call in qcow2_subcluster_zeroize(), because if I
> revert just the one in qcow2_cluster_discard(), the issue still reproduces.
> 
> Commandline for my reproducer VM [2]. The issue does not happen if I
> drop "detect-zeroes":"unmap". Note that I don't have discard-no-unref
> for the qcow2 image, so in zero_in_l2_slice(), the branch with
> qcow2_free_any_cluster() is taken. Could the conflict be related to that?
> 
> I'm still trying to figure things out and come up with a better
> reproducer, but wanted to let you know early, also because of the
> upcoming stable releases. Of course, I'd also be happy for hints/hunches
> and am happy to test suggestions!

Do you have any information about the options used with the image file?
In particular, is it using subclusters? Maybe just the 'qemu-img info'
output would already give a bit more context.

Could you already locate the actual corruption and check what the
pattern looks like? Something like zeros where we would expect data or
the other way around? Or something less clear? (If you don't know,
that's a good answer too. I know well that this kind of things is hard
to debug.)

Kevin

Hi Kevin,

Am 21.05.26 um 3:46 PM schrieb Kevin Wolf:
> Am 21.05.2026 um 14:12 hat Fiona Ebner geschrieben:
>> Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
>>> Most code in qcow2 that accesses (and potentially modifies) L2 tables
>>> does so while holding s->lock.
>>>
>>> There is one exception, which is allocating writes. They hold the lock
>>> initially while allocating clusters, but drop it for writing the guest
>>> payload before taking the lock again for updating the L2 tables. This
>>> allows concurrent requests that touch other parts of the image file to
>>> continue in parallel and is an important performance optimisation.
>>>
>>> However, this means that other requests that run while the lock is
>>> dropped for writing guest data must synchronise with the list of
>>> allocating requests in s->cluster_allocs and wait if they would overlap.
>>> For writes, this is done in handle_dependencies(), but discard and write
>>> zeros operations neglect to synchronise with s->cluster_allocs.
>>>
>>> This means that discard can free a cluster whose L2 entry will already
>>> be modified in qcow2_alloc_cluster_link_l2() by a previously started
>>> write. In the case of a pre-allocated zero cluster that is in the
>>> process of being overwritten, this means that discard can lead to a
>>> situation where the cluster is still mapped (because the write will
>>> restore the L2 entry just without the zero flag), but its refcount has
>>> been decreased, resulting in a corrupted image.
>>>
>>> Add the missing synchronisation to qcow2_cluster_discard() and
>>> qcow2_subcluster_zeroize() to fix the problem.
>>>
>>> Cc: qemu-stable@nongnu.org
>>> Reported-by: Denis V. Lunev <den@openvz.org>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>
>> we had started rolling out a build of QEMU 11 with this patch already
>> included. However, some of our users reported issues with VMs using
>> qcow2 disks soon after [0][1]. I was able to reproduce the in-guest
>> segfaults from [1] in a memory-constrained Debian 12 guest when using a
>> swap partition on the same disk. Thanks to Thomas for the hunch with
>> swap! After reverting this patch, I wasn't able to reproduce the issue
>> anymore. I do not have a better reproducer yet and am not sure about the
>> exact pattern causing the issue. It's related to the
>> wait_for_dependencies() call in qcow2_subcluster_zeroize(), because if I
>> revert just the one in qcow2_cluster_discard(), the issue still reproduces.
>>
>> Commandline for my reproducer VM [2]. The issue does not happen if I
>> drop "detect-zeroes":"unmap". Note that I don't have discard-no-unref
>> for the qcow2 image, so in zero_in_l2_slice(), the branch with
>> qcow2_free_any_cluster() is taken. Could the conflict be related to that?
>>
>> I'm still trying to figure things out and come up with a better
>> reproducer, but wanted to let you know early, also because of the
>> upcoming stable releases. Of course, I'd also be happy for hints/hunches
>> and am happy to test suggestions!
> 
> Do you have any information about the options used with the image file?
> In particular, is it using subclusters? Maybe just the 'qemu-img info'
> output would already give a bit more context.

No subclusters if I'm not missing anything. When I created the image the
output was:

Formatting '/mnt/pve/dir/images/300/vm-300-disk-0.qcow2', fmt=qcow2
cluster_size=65536 extended_l2=off preallocation=metadata
compression_type=zlib size=4510973952 lazy_refcounts=off refcount_bits=16

Our management layer doesn't log the command itself, but doing the same
operation with logging added (and 301 instead of 300):

/usr/bin/qemu-img create -o preallocation=metadata -f qcow2
/mnt/pve/dir/images/301/vm-301-disk-0.qcow2 4405248K

qemu-img info gives:

{
    "children": [
        {
            "name": "file",
            "info": {
                "children": [
                ],
                "virtual-size": 6018105344,
                "filename": "/mnt/pve/dir/images/300/vm-300-disk-0.qcow2",
                "format": "file",
                "actual-size": 3794231296,
                "format-specific": {
                    "type": "file",
                    "data": {
                        "extent-size-hint": 1048576
                    }
                },
                "dirty-flag": false
            }
        }
    ],
    "snapshots": [
        {
            "icount": 0,
            "vm-clock-nsec": 0,
            "name": "s0",
            "date-sec": 1779354489,
            "date-nsec": 415278000,
            "vm-clock-sec": 0,
            "id": "1",
            "vm-state-size": 0
        }
    ],
    "virtual-size": 4510973952,
    "filename": "/mnt/pve/dir/images/300/vm-300-disk-0.qcow2",
    "cluster-size": 65536,
    "format": "qcow2",
    "actual-size": 3794231296,
    "format-specific": {
        "type": "qcow2",
        "data": {
            "compat": "1.1",
            "compression-type": "zlib",
            "lazy-refcounts": false,
            "refcount-bits": 16,
            "corrupt": false,
            "extended-l2": false
        }
    },
    "dirty-flag": false
}

> Could you already locate the actual corruption and check what the
> pattern looks like? Something like zeros where we would expect data or
> the other way around? Or something less clear? (If you don't know,
> that's a good answer too. I know well that this kind of things is hard
> to debug.)

Unfortunately not. I can only see the symptom of memory swapped back in
being corrupt (at least that's what happens AFAIU), leading to segfaults
in various processes as well as issues with heap allocations, e.g.:
corrupted double-linked list
free(): invalid pointer

I'll write a small program which allocates memory with a fixed pattern
and regularly dumps it, maybe that works to get an idea about the
corruption.

Best Regards,
Fiona

Am 21.05.2026 um 16:18 hat Fiona Ebner geschrieben:
> Am 21.05.26 um 3:46 PM schrieb Kevin Wolf:
> > Am 21.05.2026 um 14:12 hat Fiona Ebner geschrieben:
> >> Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
> >> I'm still trying to figure things out and come up with a better
> >> reproducer, but wanted to let you know early, also because of the
> >> upcoming stable releases. Of course, I'd also be happy for hints/hunches
> >> and am happy to test suggestions!
> > 
> > Do you have any information about the options used with the image file?
> > In particular, is it using subclusters? Maybe just the 'qemu-img info'
> > output would already give a bit more context.
> 
> No subclusters if I'm not missing anything. When I created the image the
> output was:
> 
> Formatting '/mnt/pve/dir/images/300/vm-300-disk-0.qcow2', fmt=qcow2
> cluster_size=65536 extended_l2=off preallocation=metadata
> compression_type=zlib size=4510973952 lazy_refcounts=off refcount_bits=16
> 
> Our management layer doesn't log the command itself, but doing the same
> operation with logging added (and 301 instead of 300):
> 
> /usr/bin/qemu-img create -o preallocation=metadata -f qcow2
> /mnt/pve/dir/images/301/vm-301-disk-0.qcow2 4405248K
> 
> qemu-img info gives:
> [...]

Ok, looks like all default options.

> > Could you already locate the actual corruption and check what the
> > pattern looks like? Something like zeros where we would expect data or
> > the other way around? Or something less clear? (If you don't know,
> > that's a good answer too. I know well that this kind of things is hard
> > to debug.)
> 
> Unfortunately not. I can only see the symptom of memory swapped back in
> being corrupt (at least that's what happens AFAIU), leading to segfaults
> in various processes as well as issues with heap allocations, e.g.:
> corrupted double-linked list
> free(): invalid pointer
> 
> I'll write a small program which allocates memory with a fixed pattern
> and regularly dumps it, maybe that works to get an idea about the
> corruption.

AI suggests a scenario that looks like a real bug to me, though I'm not
sure if it's yours. See the reproducer below.

Basically it boils down to a non-allocating write being in flight to a
cluster that is concurrently discarded, turning the write essentially
into a host-cluster use-after-free. If you then allocate a new cluster
at the same time, the host cluster will be reused and the write that was
for a different guest cluster still writes to it.

I'm not completely sure yet what the right synchronisation mechanism
would be for this.

Anyway, as it depends on a specific pattern of discard and cluster
allocation happening while a write request is in flight, it should be
possible to use tracing to find out if anything like that is happening
in your case.

Kevin


blkdebug.conf:

[set-state]
state = "1"
event = "write_aio"
new_state = "2"

[set-state]
state = "2"
event = "cluster_alloc"
new_state = "3"


race_test.sh:

#!/bin/bash
#
# Reproducer for the wait_for_dependencies / skip_cow race in
# qcow2_subcluster_zeroize — demonstrating data corruption at an
# UNRELATED guest offset through host cluster reuse.
#
# The scenario:
#   1. Write A to a ZERO_ALLOC cluster creates l2meta. Data I/O suspended.
#   2. Write B to same cluster waits for A. Zero-write also waits for A.
#   3. A completes (cluster → NORMAL). B wakes first (FIFO), gets
#      skip_cow=true (no l2meta), starts data I/O — suspended by blkdebug.
#      Zero-write wakes, finds no deps (B invisible), frees cluster.
#   4. Write D to a DIFFERENT guest offset allocates the freed cluster.
#      D writes its data. D completes.
#   5. B resumes and writes to the same physical cluster, overwriting D.
#   6. Reading D's guest offset returns B's data. CORRUPTION.

set -e

DIR="$(cd "$(dirname "$0")" && pwd)"
QEMU_IO="${DIR}/../build/qemu-io"
QEMU_IMG="${DIR}/../build/qemu-img"
TEST_IMG="/tmp/race_test_$$.qcow2"
BLKDEBUG_CONF="${DIR}/blkdebug.conf"
LOG="/home/cursor/qemu/debug-8a8071.log"

cleanup() {
    rm -f "$TEST_IMG"
}
trap cleanup EXIT

echo "=== Creating test image ==="
"$QEMU_IMG" create -f qcow2 "$TEST_IMG" 1M

echo ""
echo "=== Preparing ZERO_ALLOC cluster at guest offset 0 ==="
"$QEMU_IO" -c "write -P 0x11 0 64k" \
            -c "write -z 0 64k" \
            "$TEST_IMG"

echo ""
echo "=== Running race reproducer ==="
#
# blkdebug.conf state machine:
#   State 1 --(write_aio)--> State 2 --(cluster_alloc)--> State 3
#
# - State 1: tagA breakpoint catches write A
# - State 2: tagB breakpoint catches write B (skip_cow write)
# - State 2→3 transition on cluster_alloc: D's allocation transitions
#   state to 3 BEFORE D fires write_aio, so D is NOT caught by tagB
#
# Sequence:
#   break write_aio tagA          -- breakpoint for state 1
#   aio_write A 0xAA 0 64k       -- suspended at tagA (state 1→2)
#   wait_break tagA
#   break write_aio tagB          -- breakpoint for state 2
#   aio_write B 0xBB 0 64k       -- waits for A (handle_dependencies)
#   aio_write -z -u 0 64k        -- waits for A (wait_for_dependencies)
#   resume tagA                   -- A completes. B wakes (skip_cow),
#                                    caught by tagB. Zero-write frees
#                                    cluster.
#   wait_break tagB               -- B suspended, cluster freed
#   write D 0xDD 64k 64k         -- D allocates the freed cluster
#                                    (cluster_alloc transitions to
#                                    state 3). D's write_aio fires at
#                                    state 3 — no breakpoint. D writes
#                                    its data and completes.
#   resume tagB                   -- B writes to the SAME physical
#                                    cluster, overwriting D's data
#   aio_flush
#
#   read -P 0xDD 64k 64k         -- EXPECTS D's data (0xDD)
#                                    GETS B's data (0xBB) → CORRUPTION

QEMU_IO_OUTPUT=$("$QEMU_IO" \
    -c "break write_aio tagA" \
    -c "aio_write -P 0xAA 0 64k" \
    -c "wait_break tagA" \
    -c "break write_aio tagB" \
    -c "aio_write -P 0xBB 0 64k" \
    -c "aio_write -z -u 0 64k" \
    -c "resume tagA" \
    -c "wait_break tagB" \
    -c "write -P 0xDD 64k 64k" \
    -c "resume tagB" \
    -c "aio_flush" \
    -c "read -vP 0xDD 64k 512" \
    -c "read -vP 0 0 512" \
    "blkdebug:${BLKDEBUG_CONF}:${TEST_IMG}" 2>&1) || true

echo "$QEMU_IO_OUTPUT"

PATTERN_FAIL=$(echo "$QEMU_IO_OUTPUT" | grep -c "Pattern verification failed" || true)
if [ "$PATTERN_FAIL" -gt 0 ]; then
    echo ""
    echo "*** DATA CORRUPTION DETECTED at guest offset 64K ***"
    echo "*** D wrote 0xDD, but reading returns B's data (0xBB)."
    echo "*** B's write to the freed+reallocated cluster corrupted"
    echo "*** an UNRELATED guest address."
fi

echo ""
echo "=== Checking image integrity (metadata) ==="
"$QEMU_IMG" check "$TEST_IMG" || true

echo ""
echo "=== Allocation map ==="
"$QEMU_IMG" map --output=json "$TEST_IMG"

echo ""
echo "=== Checking log for race evidence ==="
if [ -f "$LOG" ]; then
    echo "--- Log entries (chronological) ---"
    cat "$LOG"
    echo ""
    echo "--- Race analysis ---"
    # Extract host offsets for the skip_cow write (B) and D's write
    B_HOST=$(grep '"has_l2meta":0' "$LOG" | grep -o '"host_offset":[0-9]*' | head -1 | grep -o '[0-9]*')
    D_HOST=$(grep '"offset":65536' "$LOG" | grep -o '"host_offset":[0-9]*' | head -1 | grep -o '[0-9]*')

    echo "Write B (skip_cow, no l2meta) host_offset: $B_HOST"
    echo "Write D (different guest offset)  host_offset: $D_HOST"

    if [ -n "$B_HOST" ] && [ -n "$D_HOST" ] && [ "$B_HOST" = "$D_HOST" ]; then
        echo ""
        echo "*** CLUSTER REUSE CONFIRMED: B and D write to the same"
        echo "*** physical cluster ($B_HOST) for different guest offsets."
        echo "*** B (guest offset 0) overwrites D (guest offset 64K)."
        echo "*** Reading guest offset 64K returns B's data → CORRUPTION"
        echo "*** at an unrelated guest address."
    fi
else
    echo "No log file found at $LOG"
fi

Am 21.05.26 um 4:35 PM schrieb Kevin Wolf:
> Am 21.05.2026 um 16:18 hat Fiona Ebner geschrieben:
>> Am 21.05.26 um 3:46 PM schrieb Kevin Wolf:
>>> Am 21.05.2026 um 14:12 hat Fiona Ebner geschrieben:
>>>> Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
>>>> I'm still trying to figure things out and come up with a better
>>>> reproducer, but wanted to let you know early, also because of the
>>>> upcoming stable releases. Of course, I'd also be happy for hints/hunches
>>>> and am happy to test suggestions!
>>>
>>> Do you have any information about the options used with the image file?
>>> In particular, is it using subclusters? Maybe just the 'qemu-img info'
>>> output would already give a bit more context.
>>
>> No subclusters if I'm not missing anything. When I created the image the
>> output was:
>>
>> Formatting '/mnt/pve/dir/images/300/vm-300-disk-0.qcow2', fmt=qcow2
>> cluster_size=65536 extended_l2=off preallocation=metadata
>> compression_type=zlib size=4510973952 lazy_refcounts=off refcount_bits=16
>>
>> Our management layer doesn't log the command itself, but doing the same
>> operation with logging added (and 301 instead of 300):
>>
>> /usr/bin/qemu-img create -o preallocation=metadata -f qcow2
>> /mnt/pve/dir/images/301/vm-301-disk-0.qcow2 4405248K
>>
>> qemu-img info gives:
>> [...]
> 
> Ok, looks like all default options.
> 
>>> Could you already locate the actual corruption and check what the
>>> pattern looks like? Something like zeros where we would expect data or
>>> the other way around? Or something less clear? (If you don't know,
>>> that's a good answer too. I know well that this kind of things is hard
>>> to debug.)
>>
>> Unfortunately not. I can only see the symptom of memory swapped back in
>> being corrupt (at least that's what happens AFAIU), leading to segfaults
>> in various processes as well as issues with heap allocations, e.g.:
>> corrupted double-linked list
>> free(): invalid pointer
>>
>> I'll write a small program which allocates memory with a fixed pattern
>> and regularly dumps it, maybe that works to get an idea about the
>> corruption.
> 
> AI suggests a scenario that looks like a real bug to me, though I'm not
> sure if it's yours. See the reproducer below.
> 
> Basically it boils down to a non-allocating write being in flight to a
> cluster that is concurrently discarded, turning the write essentially
> into a host-cluster use-after-free. If you then allocate a new cluster
> at the same time, the host cluster will be reused and the write that was
> for a different guest cluster still writes to it.
> 
> I'm not completely sure yet what the right synchronisation mechanism
> would be for this.
> 
> Anyway, as it depends on a specific pattern of discard and cluster
> allocation happening while a write request is in flight, it should be
> possible to use tracing to find out if anything like that is happening
> in your case.

I will try to do tracing. With the following program, I see that the
corrupt memory reads back as zeroes:

> #include <stdbool.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> 
> #define SIZE 1024
> #define COUNT 100 * 1024
> 
> uint8_t data[COUNT * SIZE];
> 
> int main(void) {
>     while (true) {
>         for (uint32_t i = 0; i < COUNT; i++) {
>             memset(data + i * SIZE, (i % 100) + 1, SIZE);
>         }
>         sleep(10);
>         for (uint32_t i = 0; i < COUNT; i++) {
>             for (uint32_t j = 0; j < SIZE; j++) {
>                 if (data[i * SIZE + j] != (i % 100) + 1) {
>                     uint32_t start = j;
>                     uint32_t corrupt_val = data[i * SIZE + j];
>                     while (j + 1 < SIZE && data[i * SIZE + j + 1] == corrupt_val) {
>                         j++;
>                     }
>                     printf("corrupt block %u byte range %u - %u: val %u expected %u\n",
>                            i, start, j, corrupt_val, (i % 100) + 1);
>                 }
>             }
>         }
>     }
>     return 0;
> }

Example output:

> corrupt block 21495 byte range 928 - 1023: val 0 expected 96
> corrupt block 21496 byte range 0 - 1023: val 0 expected 97
> corrupt block 21497 byte range 0 - 1023: val 0 expected 98
> corrupt block 21498 byte range 0 - 1023: val 0 expected 99
> corrupt block 21499 byte range 0 - 1023: val 0 expected 100
> corrupt block 21500 byte range 0 - 1023: val 0 expected 1
> corrupt block 21501 byte range 0 - 1023: val 0 expected 2
> corrupt block 21502 byte range 0 - 1023: val 0 expected 3
> corrupt block 21503 byte range 0 - 1023: val 0 expected 4
> corrupt block 21504 byte range 0 - 1023: val 0 expected 5
> corrupt block 21505 byte range 0 - 1023: val 0 expected 6
> corrupt block 21506 byte range 0 - 1023: val 0 expected 7
> corrupt block 21507 byte range 0 - 1023: val 0 expected 8
> corrupt block 21508 byte range 0 - 1023: val 0 expected 9
> corrupt block 21509 byte range 0 - 1023: val 0 expected 10
> corrupt block 21510 byte range 0 - 1023: val 0 expected 11
> corrupt block 21511 byte range 0 - 927: val 0 expected 12
> corrupt block 22727 byte range 928 - 1023: val 0 expected 28
> corrupt block 22728 byte range 0 - 1023: val 0 expected 29
> corrupt block 22729 byte range 0 - 1023: val 0 expected 30
> corrupt block 22730 byte range 0 - 1023: val 0 expected 31
> corrupt block 22731 byte range 0 - 1023: val 0 expected 32
> corrupt block 22732 byte range 0 - 1023: val 0 expected 33
> corrupt block 22733 byte range 0 - 1023: val 0 expected 34
> corrupt block 22734 byte range 0 - 1023: val 0 expected 35
> corrupt block 22735 byte range 0 - 1023: val 0 expected 36
> corrupt block 22736 byte range 0 - 1023: val 0 expected 37
> corrupt block 22737 byte range 0 - 1023: val 0 expected 38
> corrupt block 22738 byte range 0 - 1023: val 0 expected 39
> corrupt block 22739 byte range 0 - 1023: val 0 expected 40
> corrupt block 22740 byte range 0 - 1023: val 0 expected 41
> corrupt block 22741 byte range 0 - 1023: val 0 expected 42
> corrupt block 22742 byte range 0 - 1023: val 0 expected 43
> corrupt block 22743 byte range 0 - 1023: val 0 expected 44
> corrupt block 22744 byte range 0 - 1023: val 0 expected 45
> corrupt block 22745 byte range 0 - 1023: val 0 expected 46
> corrupt block 22746 byte range 0 - 1023: val 0 expected 47
> corrupt block 22747 byte range 0 - 1023: val 0 expected 48
> corrupt block 22748 byte range 0 - 1023: val 0 expected 49
> corrupt block 22749 byte range 0 - 1023: val 0 expected 50
> corrupt block 22750 byte range 0 - 1023: val 0 expected 51
> corrupt block 22751 byte range 0 - 927: val 0 expected 52
> corrupt block 23451 byte range 928 - 1023: val 0 expected 52
> corrupt block 23452 byte range 0 - 1023: val 0 expected 53
> corrupt block 23453 byte range 0 - 1023: val 0 expected 54
> corrupt block 23454 byte range 0 - 1023: val 0 expected 55
> corrupt block 23455 byte range 0 - 1023: val 0 expected 56
> corrupt block 23456 byte range 0 - 1023: val 0 expected 57
> corrupt block 23457 byte range 0 - 1023: val 0 expected 58
> corrupt block 23458 byte range 0 - 1023: val 0 expected 59
> corrupt block 23459 byte range 0 - 1023: val 0 expected 60
> corrupt block 23460 byte range 0 - 1023: val 0 expected 61
> corrupt block 23461 byte range 0 - 1023: val 0 expected 62
> corrupt block 23462 byte range 0 - 1023: val 0 expected 63
> corrupt block 23463 byte range 0 - 1023: val 0 expected 64
> corrupt block 23464 byte range 0 - 1023: val 0 expected 65
> corrupt block 23465 byte range 0 - 1023: val 0 expected 66
> corrupt block 23466 byte range 0 - 1023: val 0 expected 67
> corrupt block 23467 byte range 0 - 1023: val 0 expected 68
> corrupt block 23468 byte range 0 - 1023: val 0 expected 69
> corrupt block 23469 byte range 0 - 1023: val 0 expected 70
> corrupt block 23470 byte range 0 - 1023: val 0 expected 71
> corrupt block 23471 byte range 0 - 1023: val 0 expected 72
> corrupt block 23472 byte range 0 - 1023: val 0 expected 73
> corrupt block 23473 byte range 0 - 1023: val 0 expected 74
> corrupt block 23474 byte range 0 - 1023: val 0 expected 75
> corrupt block 23475 byte range 0 - 1023: val 0 expected 76
> corrupt block 23476 byte range 0 - 1023: val 0 expected 77
> corrupt block 23477 byte range 0 - 1023: val 0 expected 78
> corrupt block 23478 byte range 0 - 1023: val 0 expected 79
> corrupt block 23479 byte range 0 - 1023: val 0 expected 80
> corrupt block 23480 byte range 0 - 1023: val 0 expected 81
> corrupt block 23481 byte range 0 - 1023: val 0 expected 82
> corrupt block 23482 byte range 0 - 1023: val 0 expected 83
> corrupt block 23483 byte range 0 - 1023: val 0 expected 84
> corrupt block 23484 byte range 0 - 1023: val 0 expected 85
> corrupt block 23485 byte range 0 - 1023: val 0 expected 86
> corrupt block 23486 byte range 0 - 1023: val 0 expected 87
> corrupt block 23487 byte range 0 - 927: val 0 expected 88

Am 21.05.26 um 17:14 schrieb Fiona Ebner:
> Am 21.05.26 um 4:35 PM schrieb Kevin Wolf:
>> Anyway, as it depends on a specific pattern of discard and cluster
>> allocation happening while a write request is in flight, it should be
>> possible to use tracing to find out if anything like that is happening
>> in your case.
> 
> I will try to do tracing. With the following program, I see that the
> corrupt memory reads back as zeroes:
Hmm, looking at qcow2_co_pwrite_zeroes(): could the cluster state
observed before wait_for_dependencies() drops s->lock differ from what
qcow2_subcluster_zeroize() then operates on?

Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
> @@ -2000,6 +2028,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
>      int64_t cleared;
>      int ret;
>  
> +    /*
> +     * If we're touching a cluster for which allocating writes are in flight,
> +     * wait for them to complete to avoid conflicting metadata updates.
> +     *
> +     * We don't need to allocate a QCowL2Meta for the discard operation because
> +     * s->lock is held for the duration of the whole operation.

The caller in qcow2_make_empty() does not acquire s->lock AFAICS. Can
that be a problem?

> +     */
> +    wait_for_dependencies(bs, offset, bytes);
> +
>      /* Caller must pass aligned values, except at image end */
>      assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
>      assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||

Am 29.04.2026 um 17:28 hat Fiona Ebner geschrieben:
> Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
> > @@ -2000,6 +2028,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
> >      int64_t cleared;
> >      int ret;
> >  
> > +    /*
> > +     * If we're touching a cluster for which allocating writes are in flight,
> > +     * wait for them to complete to avoid conflicting metadata updates.
> > +     *
> > +     * We don't need to allocate a QCowL2Meta for the discard operation because
> > +     * s->lock is held for the duration of the whole operation.
> 
> The caller in qcow2_make_empty() does not acquire s->lock AFAICS. Can
> that be a problem?

This is the case the comment inside wait_for_dependencies() is about:

    /*
     * Discard has some non-coroutine callers (creating internal snapshots and
     * make empty). They are calling from qemu-img or in a drained section, so
     * we know that no writes can be in progress.
     */

So it should be fine in practice because we know a bigger hammer is
already in use for synchronisation.

Kevin

> > +     */
> > +    wait_for_dependencies(bs, offset, bytes);
> > +
> >      /* Caller must pass aligned values, except at image end */
> >      assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
> >      assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||

Am 12.05.26 um 2:21 PM schrieb Kevin Wolf:
> Am 29.04.2026 um 17:28 hat Fiona Ebner geschrieben:
>> Am 27.04.26 um 7:04 PM schrieb Kevin Wolf:
>>> @@ -2000,6 +2028,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
>>>      int64_t cleared;
>>>      int ret;
>>>  
>>> +    /*
>>> +     * If we're touching a cluster for which allocating writes are in flight,
>>> +     * wait for them to complete to avoid conflicting metadata updates.
>>> +     *
>>> +     * We don't need to allocate a QCowL2Meta for the discard operation because
>>> +     * s->lock is held for the duration of the whole operation.
>>
>> The caller in qcow2_make_empty() does not acquire s->lock AFAICS. Can
>> that be a problem?
> 
> This is the case the comment inside wait_for_dependencies() is about:
> 
>     /*
>      * Discard has some non-coroutine callers (creating internal snapshots and
>      * make empty). They are calling from qemu-img or in a drained section, so
>      * we know that no writes can be in progress.
>      */
> 
> So it should be fine in practice because we know a bigger hammer is
> already in use for synchronisation.

Ah sorry, I missed that. Ignore me then.

Best Regards,
Fiona