Re: [Qemu-devel] [PATCH v2 7/7] block/qcow2-refcount: fix out-of-file L2 entries to be read-as-zero

[Vladimir Sementsov-Ogievskiy]

17.08.2018 15:22, Vladimir Sementsov-Ogievskiy wrote:
> Rewrite corrupted L2 table entry, which reference space out of
> underlying file.
>
> Make this L2 table entry read-as-all-zeros without any allocation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   block/qcow2-refcount.c | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 3c004e5bfe..3de3768a3c 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1720,8 +1720,30 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>               /* Mark cluster as used */
>               csize = (((l2_entry >> s->csize_shift) & s->csize_mask) + 1) *
>                       BDRV_SECTOR_SIZE;
> +            if (csize > s->cluster_size) {
> +                ret = fix_l2_entry_to_zero(
> +                        bs, res, fix, l2_offset, i, active,
> +                        "compressed cluster larger than cluster: size 0x%"
> +                        PRIx64, csize);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                continue;
> +            }
> +
>               coffset = l2_entry & s->cluster_offset_mask &
>                         ~(BDRV_SECTOR_SIZE - 1);
> +            if (coffset >= bdrv_getlength(bs->file->bs)) {
> +                ret = fix_l2_entry_to_zero(
> +                        bs, res, fix, l2_offset, i, active,
> +                        "compressed cluster out of file: offset 0x%" PRIx64,
> +                        coffset);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                continue;
> +            }
> +
>               ret = qcow2_inc_refcounts_imrt(bs, res,
>                                              refcount_table, 
> refcount_table_size,
>                                              coffset, csize);
> @@ -1748,6 +1770,16 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>           {
>               uint64_t offset = l2_entry & L2E_OFFSET_MASK;
>   
> +            if (offset >= bdrv_getlength(bs->file->bs)) {
> +                ret = fix_l2_entry_to_zero(
> +                        bs, res, fix, l2_offset, i, active,
> +                        "cluster out of file: offset 0x%" PRIx64, offset);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                continue;
> +            }
> +
>               if (flags & CHECK_FRAG_INFO) {
>                   res->bfi.allocated_clusters++;
>                   if (next_contiguous_offset &&

hmm, interesting question here: in case of misaligned l2 entry, we zero 
it out only for QCOW2_CLUSTER_ZERO_ALLOC, but not for normal clusters? 
Why? I think it is ok to mark as zero misaligned normal cluster l2 
entry, otherwise we'll have fatal corruption on any operation to this 
cluster.

-- 
Best regards,
Vladimir