Re: [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()

[Vladimir Sementsov-Ogievskiy]

03.04.2020 20:40, Eric Blake wrote:
> On 4/3/20 11:57 AM, Alberto Garcia wrote:
> > When issuing a compressed write request the number of bytes must be a
> > multiple of the cluster size.
> >
> > 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 <address@hidden>
> > ---
> >   block/qcow2.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Eric Blake <address@hidden>
>
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 2bb536b014..923ad428f0 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -4345,7 +4345,7 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
> >           return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, 
> > NULL);
> >       }
> > -    if (offset_into_cluster(s, offset)) {
> > +    if (offset_into_cluster(s, offset | bytes)) {
> >           return -EINVAL;
> >       }

This will break compressed write to the tail of unaligned to cluster_size 
image, which is possible as I understand.

Check should make an exception for this case, like the assertion does:

len = bdrv_getlength(bs);
if (offset_into_cluster(s, offset) || (offset_into_cluster(bytes) && offset + 
bytes != len)) {
  return -EINVAL;
}


--
Best regards,
Vladimir