Skip to content
Snippets Groups Projects
  • Maxim Levitsky's avatar
    c8bf9a91
    qcow2: Fix corruption on write_zeroes with MAY_UNMAP · c8bf9a91
    Maxim Levitsky authored
    
    Commit 205fa507 ("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: 205fa507 ("qcow2: Add subcluster support to zero_in_l2_slice()")
    Signed-off-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
    [ kwolf: Fixed to restore the correct original order from before
      205fa507; added comments like in discard_in_l2_slice(). ]
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    Message-Id: <20201124092815.39056-1-kwolf@redhat.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    c8bf9a91
    History
    qcow2: Fix corruption on write_zeroes with MAY_UNMAP
    Maxim Levitsky authored
    
    Commit 205fa507 ("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: 205fa507 ("qcow2: Add subcluster support to zero_in_l2_slice()")
    Signed-off-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
    [ kwolf: Fixed to restore the correct original order from before
      205fa507; added comments like in discard_in_l2_slice(). ]
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    Message-Id: <20201124092815.39056-1-kwolf@redhat.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>