Re: [PATCH for-5.2 v2] qcow2: Fix corruption on write_zeroes with MAY_UNMAP

[Alberto Garcia]

On Tue 24 Nov 2020 10:28:15 AM CET, Kevin Wolf wrote:
> From: Maxim Levitsky <mlevitsk@redhat.com>
>
> Commit 205fa50750 ("qcow2: Add subcluster support to zero_in_l2_slice()")
> introduced a subtle change to code in zero_in_l2_slice:
>
> It swapped the order of
>
> 1. qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
> 2. set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
> 3. qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
>
> To
>
> 1. qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
> 2. qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
> 3. set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
>
> It seems harmless, however the call to qcow2_free_any_clusters can
> trigger a cache flush which can mark the L2 table as clean, and
> assuming that this was the last write to it, a stale version of it
> will remain on the disk.
>
> Now we have a valid L2 entry pointing to a freed cluster. Oops.
>
> Fixes: 205fa50750 ("qcow2: Add subcluster support to zero_in_l2_slice()")
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> [ kwolf: Fixed to restore the correct original order from before
>   205fa50750; added comments like in discard_in_l2_slice(). ]

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto