[sheepdog] [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()
eblake at redhat.com
Tue Jun 23 18:37:42 CEST 2020
On 6/23/20 5:20 AM, Vladimir Sementsov-Ogievskiy wrote:
> 11.05.2020 21:34, Eric Blake wrote:
>> On 5/11/20 12:17 PM, Alberto Garcia wrote:
>>> On Thu 30 Apr 2020 01:10:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>>> compute 'int tail' via % 'int alignment' - safe
>>> tail = (offset + bytes) % alignment;
>>> both are int64_t, no chance of overflow here?
>> Good question - I know several places check that offset+bytes does not
>> overflow, but did not specifically audit if this one does. Adding an
>> assert() in this function may be easier than trying to prove all
>> callers pass in safe values.
> Hm, it's preexisting, as int64_t + int may overflow as well. Strange,
> but I don't see overflow check neither in blk_check_byte_request nor in
> bdrv_check_byte_request. Only discard, which recently dropped call of
> bdrv_check_byte_request() has this check.
In fact, iotest 197 (see commit 461743390) is an instance of testing for
a bug where we overflowed INT_MAX due to rounding up to cluster size,
even with a transaction request smaller than limits.
> I can add a patch for overflow check in blk_check_byte_request and
> bdrv_check_byte_request.. But what about alignment? There may be
> requests, for which bytes + offset doesn't overflow, but do overflow
> after aligning up. Refactor bdrv_pad_request() to return an error if we
> can't pad request due to overflow?
The only cases where int64_t + int can overflow due to rounding up for
alignment are when the file size is extremely close to 2^63 bytes
already. The easiest fix is to reject opening a file that reports a
size that would overflow when rounded up to alignment (that is, if size
> INT64_MAX - alignment, we should refuse to proceed). Such images
will never occur for actual disk images (because that is really a LOT of
storage), but are possible over things like NBD (in fact, nbdkit has
intentionally made it easy to provoke boundary testing near 2^63 bytes,
and is already aware that anything larger than 2^63-512 is problematic
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
More information about the sheepdog