Re: [PATCH v3] qcow2: Avoid integer wraparound in qcow2_co_truncate()

[Kevin Wolf <kwolf@redhat.com>]

Am 05.05.2020 um 10:54 hat Kevin Wolf geschrieben:
> Am 04.05.2020 um 19:07 hat Alberto Garcia geschrieben:
> > On Mon 04 May 2020 06:01:19 PM CEST, Eric Blake wrote:
> > >> +_supported_fmt qcow2
> > >> +_supported_proto file
> > >
> > > Do we have to limit it to qcow2 and file?  Yes, it's testing a bugfix
> > > for qcow2, but are there other formats that it doesn't hurt to have
> > > the extra testing?
> > 
> > It doesn't work with any other format at the moment (meaning: reading
> > the tail of the image after growing it returns the data from the backing
> > file).
> > 
> > Also, it seems that qemu-img's -F does not work with other formats
> > either.
> > 
> > > Also, I don't see anything preventing this from working with non-file
> > > protocol.
> > 
> > Right, that can be updated I guess (whoever commits this, feel free to
> > do it).
> 
> I don't know for which protocols it works. I know that qcow2 over nbd
> doesn't work.
> 
> But I think there is a more important problem with the test: It seems to
> pass even with old binaries that don't have the fix. Is this only on my
> system or do you get the same?

Ah, I do get the overflow in the calculation of the length for
qcow2_cluster_zeroize(), but size_to_clusters() inside the function
overflows back the other direction, so this ends up with
nb_clusters = 0 and we don't do anything bad.

We could probably trigger a bad case with data_file_raw=on, but then we
don't have a backing file, so nothing sets BDRV_REQ_ZERO_WRITE.

So I guess the bug isn't even really testable, but we just add the test
in case something else in the same scenario breaks?

Kevin