[PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Alberto Garcia]

When issuing a compressed write request the number of bytes must be a
multiple of the cluster size or reach the end of the last cluster.

With the current code such requests are allowed and we hit an
assertion:

   $ qemu-img create -f qcow2 img.qcow2 1M
   $ qemu-io -c 'write -c 0 32k' img.qcow2

   qemu-io: block/qcow2.c:4257: qcow2_co_pwritev_compressed_task:
   Assertion `bytes == s->cluster_size || (bytes < s->cluster_size &&
              (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed.
   Aborted

This patch fixes a regression introduced in 0d483dce38

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2bb536b014..587cf51948 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4349,6 +4349,11 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
         return -EINVAL;
     }
 
+    if (offset_into_cluster(s, bytes) &&
+        (offset + bytes) != (bs->total_sectors << BDRV_SECTOR_BITS)) {
+        return -EINVAL;
+    }
+
     while (bytes && aio_task_pool_status(aio) == 0) {
         uint64_t chunk_size = MIN(bytes, s->cluster_size);
 
-- 
2.20.1

Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Andrey Shinkevich]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

________________________________
From: Alberto Garcia <berto@igalia.com>
Sent: Monday, April 6, 2020 5:34 PM
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Cc: Alberto Garcia <berto@igalia.com>; qemu-block@nongnu.org <qemu-block@nongnu.org>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>; Max Reitz <mreitz@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Pavel Butsykin <pbutsykin@virtuozzo.com>
Subject: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()


[-- Attachment #2: Type: text/html, Size: 1733 bytes --]

Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Alberto Garcia]

I forgot to add the "for-5.0" tag in the subject, do I need to resend
the patch?

Berto

Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Andrey Shinkevich]

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

I wouldn't mind either.

Andrey

________________________________
From: Alberto Garcia <berto@igalia.com>
Sent: Monday, April 6, 2020 7:08 PM
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Cc: qemu-block@nongnu.org <qemu-block@nongnu.org>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>; Max Reitz <mreitz@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Pavel Butsykin <pbutsykin@virtuozzo.com>
Subject: Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

I forgot to add the "for-5.0" tag in the subject, do I need to resend
the patch?

Berto

[-- Attachment #2: Type: text/html, Size: 1765 bytes --]

Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Vladimir Sementsov-Ogievskiy]

06.04.2020 17:34, Alberto Garcia wrote:
> When issuing a compressed write request the number of bytes must be a
> multiple of the cluster size or reach the end of the last cluster.
> 
> With the current code such requests are allowed and we hit an
> assertion:
> 
>     $ qemu-img create -f qcow2 img.qcow2 1M
>     $ qemu-io -c 'write -c 0 32k' img.qcow2
> 
>     qemu-io: block/qcow2.c:4257: qcow2_co_pwritev_compressed_task:
>     Assertion `bytes == s->cluster_size || (bytes < s->cluster_size &&
>                (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed.
>     Aborted
> 
> This patch fixes a regression introduced in 0d483dce38
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   block/qcow2.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2bb536b014..587cf51948 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -4349,6 +4349,11 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
>           return -EINVAL;
>       }
>   
> +    if (offset_into_cluster(s, bytes) &&
> +        (offset + bytes) != (bs->total_sectors << BDRV_SECTOR_BITS)) {
> +        return -EINVAL;
> +    }
> +
>       while (bytes && aio_task_pool_status(aio) == 0) {
>           uint64_t chunk_size = MIN(bytes, s->cluster_size);
>   
> 


-- 
Best regards,
Vladimir

Re: [PATCH v2] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 907 bytes --]

On 06.04.20 16:34, Alberto Garcia wrote:
> When issuing a compressed write request the number of bytes must be a
> multiple of the cluster size or reach the end of the last cluster.
> 
> With the current code such requests are allowed and we hit an
> assertion:
> 
>    $ qemu-img create -f qcow2 img.qcow2 1M
>    $ qemu-io -c 'write -c 0 32k' img.qcow2
> 
>    qemu-io: block/qcow2.c:4257: qcow2_co_pwritev_compressed_task:
>    Assertion `bytes == s->cluster_size || (bytes < s->cluster_size &&
>               (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed.
>    Aborted
> 
> This patch fixes a regression introduced in 0d483dce38
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2.c | 5 +++++
>  1 file changed, 5 insertions(+)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]