Re: [PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential integer overflow

[Max Reitz]

[Cc-ing Stefan]

On 09.11.20 16:05, Peter Maydell wrote:
> In vu_blk_discard_write_zeroes(), we read a 32-bit sector count from
> the descriptor and convert it to a 64-bit byte count. Coverity warns
> that the left shift is done with 32-bit arithmetic so it might
> overflow before the conversion to 64-bit happens. Add a cast to
> avoid this.

This will silence Coverity, but both functions to which range[1] is then 
passed (blk_co_pdiscard() and blk_co_pwrite_zeroes()) only accept ints 
there, so this would only move the overflow to the function call.

Shouldn’t we verify that the number of sectors is in range and return an 
error if it isn’t?  (The same probably goes for the starting sector, 
then, too.)

Max

> Fixes: Coverity CID 1435956
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Tested with 'make check' and 'make check-acceptance' only.
> ---
>   block/export/vhost-user-blk-server.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/export/vhost-user-blk-server.c 
> b/block/export/vhost-user-blk-server.c
> index 62672d1cb95..e5749451e65 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
> @@ -70,7 +70,7 @@ vu_blk_discard_write_zeroes(BlockBackend *blk, struct iovec 
> *iov,
>       }
>   
>       uint64_t range[2] = { le64_to_cpu(desc.sector) << 9,
> -                          le32_to_cpu(desc.num_sectors) << 9 };
> +                          (uint64_t)le32_to_cpu(desc.num_sectors) << 9 };
>       if (type == VIRTIO_BLK_T_DISCARD) {
>           if (blk_co_pdiscard(blk, range[0], range[1]) == 0) {
>               return 0;