Re: [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Vladimir Sementsov-Ogievskiy]

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.

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?

--
Best regards,
Vladimir