On Thu 22 Feb 2018 08:02:44 PM CET, Eric Blake wrote: > On 02/22/2018 10:23 AM, Alberto Garcia wrote: >> On Thu 22 Feb 2018 04:59:22 PM CET, Eric Blake wrote: >>> sector_offset = coffset & 511; >>> csize = nb_csectors * 512 - sector_offset; >> [...] >>> + assert(csize < 2 * s->cluster_size); >> >> I think it should be <= >> >> If sector_offset is 0 and nb_csector is the maximum allowed value then >> csize is exactly 2 * s->cluster_size bytes. > > Sigh, yes you're right. I was thinking that "qemu sets csize to a > maximum of s->cluster_size, but only when sector_offset is not 0" - but > as long as we're dealing with externally-produced images, sector_offset > can be 0 at the same time as providing all 1s to the field. So I did > indeed have an off-by-one. > > Perhaps the maintainer can fix it up, instead of me spinning a v4?
That would work for me, but note that this part of the commit message also needs to be updated: In fact, the qcow2 spec permits an all-ones sector count, plus 511 bytes from the sector containing the initial offset, for a maximum read of nearly 2 full clusters; With those two things corrected, Reviewed-by: Alberto Garcia <be...@igalia.com> Berto
