All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/1] Block layer patches for 5.2.0-rc3
@ 2020-11-24 14:24 Kevin Wolf
  2020-11-24 14:24 ` [PULL 1/1] qcow2: Fix corruption on write_zeroes with MAY_UNMAP Kevin Wolf
  2020-11-24 21:07 ` [PULL 0/1] Block layer patches for 5.2.0-rc3 Peter Maydell
  0 siblings, 2 replies; 3+ messages in thread
From: Kevin Wolf @ 2020-11-24 14:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

The following changes since commit 23895cbd82be95428e90168b12e925d0d3ca2f06:

  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201123.0' into staging (2020-11-23 18:51:13 +0000)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to c8bf9a9169db094aaed680cdba570758c0dc18b9:

  qcow2: Fix corruption on write_zeroes with MAY_UNMAP (2020-11-24 11:29:41 +0100)

----------------------------------------------------------------
Patches for 5.2.0-rc3:

- qcow2: Fix corruption on write_zeroes with MAY_UNMAP

----------------------------------------------------------------
Maxim Levitsky (1):
      qcow2: Fix corruption on write_zeroes with MAY_UNMAP

 block/qcow2-cluster.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PULL 1/1] qcow2: Fix corruption on write_zeroes with MAY_UNMAP
  2020-11-24 14:24 [PULL 0/1] Block layer patches for 5.2.0-rc3 Kevin Wolf
@ 2020-11-24 14:24 ` Kevin Wolf
  2020-11-24 21:07 ` [PULL 0/1] Block layer patches for 5.2.0-rc3 Peter Maydell
  1 sibling, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2020-11-24 14:24 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

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(). ]
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201124092815.39056-1-kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 485b4cb92e..bd0597842f 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2010,14 +2010,17 @@ static int zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
             continue;
         }
 
+        /* First update L2 entries */
         qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
-        if (unmap) {
-            qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST);
-        }
         set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry);
         if (has_subclusters(s)) {
             set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap);
         }
+
+        /* Then decrease the refcount */
+        if (unmap) {
+            qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST);
+        }
     }
 
     qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PULL 0/1] Block layer patches for 5.2.0-rc3
  2020-11-24 14:24 [PULL 0/1] Block layer patches for 5.2.0-rc3 Kevin Wolf
  2020-11-24 14:24 ` [PULL 1/1] qcow2: Fix corruption on write_zeroes with MAY_UNMAP Kevin Wolf
@ 2020-11-24 21:07 ` Peter Maydell
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2020-11-24 21:07 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block

On Tue, 24 Nov 2020 at 14:25, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 23895cbd82be95428e90168b12e925d0d3ca2f06:
>
>   Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201123.0' into staging (2020-11-23 18:51:13 +0000)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to c8bf9a9169db094aaed680cdba570758c0dc18b9:
>
>   qcow2: Fix corruption on write_zeroes with MAY_UNMAP (2020-11-24 11:29:41 +0100)
>
> ----------------------------------------------------------------
> Patches for 5.2.0-rc3:
>
> - qcow2: Fix corruption on write_zeroes with MAY_UNMAP
>
> ----------------------------------------------------------------
> Maxim Levitsky (1):
>       qcow2: Fix corruption on write_zeroes with MAY_UNMAP


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-11-24 21:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-24 14:24 [PULL 0/1] Block layer patches for 5.2.0-rc3 Kevin Wolf
2020-11-24 14:24 ` [PULL 1/1] qcow2: Fix corruption on write_zeroes with MAY_UNMAP Kevin Wolf
2020-11-24 21:07 ` [PULL 0/1] Block layer patches for 5.2.0-rc3 Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.