On 01/31/2017 07:54 PM, Max Reitz wrote: >>> Another thing: I'm not quite positive whether qcow2_cluster_zeroize() >>> can actually handle a byte count greater than INT_MAX. If you could pass >>> such a number to it, the multiplication "ret * s->cluster_size" might >>> overflow. It's only caller qcow2_co_pwrite_zeroes() will never exceed >>> that limit, but maybe this should be made clear somewhere -- either by >>> making the parameter an int instead of a uint64_t or by asserting it in >>> the function. >> >> I'd lean towards an assertion, especially since it would be nice to >> someday unify all the internal block interfaces to get to a 64-bit >> interface wherever possible > > Kevin once said "We should write code that makes sense today, not code > that will make sense some day in the future." Or something like that. :-)
Hmm. I looked a bit further. The calculation of "ret * s->cluster_size" is bounded by the maximum value of the earlier "ret = zero_single_l2()", which is limited to the number of clusters in a single L2 table. But when you have an image with 2M clusters, a single L2 page thus covers 2M/8 (or 262144) clusters, but that is in turn much larger than INT_MAX. My v5 will fix things to track the int return value separately from the 64-bit crawl through zero_single_l2() (in other words, quit overloading 'ret' to serve two purposes). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
