On Wed, 3 Jul 2024 at 23:48, Michael S. Tsirkin <m...@redhat.com> wrote:
>
> From: Stefano Garzarella <sgarz...@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é <phi...@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> Acked-by: Stefan Hajnoczi <stefa...@redhat.com>
> Reviewed-by: David Hildenbrand <da...@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarz...@redhat.com>
> Message-Id: <20240618100447.145697-1-sgarz...@redhat.com>
> Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> Signed-off-by: Michael S. Tsirkin <m...@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
