Re: [PULL v3 52/85] contrib/vhost-user-*: use QEMU bswap helper functions

[Peter Maydell]

On Wed, 3 Jul 2024 at 23:48, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> Let's replace the calls to le*toh() and htole*() with qemu/bswap.h
> helpers to make the code more portable.
>
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> Message-Id: <20240618100447.145697-1-sgarzare@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  contrib/vhost-user-blk/vhost-user-blk.c |  9 +++++----
>  contrib/vhost-user-input/main.c         | 16 ++++++++--------
>  2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c 
> b/contrib/vhost-user-blk/vhost-user-blk.c
> index a8ab9269a2..9492146855 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -16,6 +16,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/bswap.h"
>  #include "standard-headers/linux/virtio_blk.h"
>  #include "libvhost-user-glib.h"
>
> @@ -194,8 +195,8 @@ vub_discard_write_zeroes(VubReq *req, struct iovec *iov, 
> uint32_t iovcnt,
>      #if defined(__linux__) && defined(BLKDISCARD) && defined(BLKZEROOUT)
>      VubDev *vdev_blk = req->vdev_blk;
>      desc = buf;
> -    uint64_t range[2] = { le64toh(desc->sector) << 9,
> -                          le32toh(desc->num_sectors) << 9 };
> +    uint64_t range[2] = { le64_to_cpu(desc->sector) << 9,
> +                          le32_to_cpu(desc->num_sectors) << 9 };

Hi; Coverity points out that this does a 32-bit shift, not a
64-bit one, so it could unintentionally chop the high bits off
if desc->num_sectors is big enough (CID 1549454).
We could fix this by making it
    (uint64_t)le32_to_cpu(desc->num_sectors) << 9
I think.

(It looks like the issue was already there before, so
Coverity has just noticed it because of the code change here.)

thanks
-- PMM