Re: [PATCH] contrib/vhost-user-blk: fix overflowing expression

[Peter Maydell]

On Fri, 12 Jul 2024 at 16:39, Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> Coverity reported:
>
>   >>>     CID 1549454:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
>   >>>     Potentially overflowing expression
>           "le32_to_cpu(desc->num_sectors) << 9" with type "uint32_t"
>           (32 bits, unsigned) is evaluated using 32-bit arithmetic, and
>           then used in a context that expects an expression of type
>           "uint64_t" (64 bits, unsigned).
>   199                               le32_to_cpu(desc->num_sectors) << 9 };
>
> Coverity noticed this issue after commit ab04420c3 ("contrib/vhost-user-*:
> use QEMU bswap helper functions"), but it was pre-existing and introduced
> from the beginning by commit caa1ee4313 ("vhost-user-blk: add
> discard/write zeroes features support").
>
> Explicitly cast the 32-bit value before the shift to fix this issue.
>
> Fixes: Coverity CID 1549454
> Fixes: 5ab04420c3 ("contrib/vhost-user-*: use QEMU bswap helper functions")
> Fixes: caa1ee4313 ("vhost-user-blk: add discard/write zeroes features 
> support")
> Cc: changpeng.liu@intel.com
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  contrib/vhost-user-blk/vhost-user-blk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c 
> b/contrib/vhost-user-blk/vhost-user-blk.c
> index 9492146855..6cc18a1c04 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -196,7 +196,7 @@ vub_discard_write_zeroes(VubReq *req, struct iovec *iov, 
> uint32_t iovcnt,
>      VubDev *vdev_blk = req->vdev_blk;
>      desc = buf;
>      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 (ioctl(vdev_blk->blk_fd, BLKDISCARD, range) == 0) {
>              g_free(buf);
> --

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM