Re: [PATCH v1] vfio: Fix CID 1458134 in vfio_register_ram_discard_listener()

[Pankaj Gupta <pankaj.gupta.linux@gmail.com>]

>   CID 1458134:  Integer handling issues  (BAD_SHIFT)
>     In expression "1 << ctz64(container->pgsizes)", left shifting by more
>     than 31 bits has undefined behavior.  The shift amount,
>     "ctz64(container->pgsizes)", is 64.
>
> Commit 5e3b981c330c ("vfio: Support for RamDiscardManager in the !vIOMMU
> case") added an assertion that our granularity is at least as big as the
> page size.
>
> Although unlikely, we could have a page size that does not fit into
> 32 bit. In that case, we'd try shifting by more than 31 bit.
>
> Let's use 1ULL instead and make sure we're not shifting by more than 63
> bit by asserting that any bit in container->pgsizes is set.
>
> Fixes: CID 1458134
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Auger Eric <eric.auger@redhat.com>
> Cc: Wei Yang <richard.weiyang@linux.alibaba.com>
> Cc: teawater <teawaterz@linux.alibaba.com>
> Cc: Marek Kedzierski <mkedzier@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/vfio/common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 3f0d111360..8728d4d5c2 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -783,7 +783,8 @@ static void vfio_register_ram_discard_listener(VFIOContainer *container,
>                                                                  section->mr);
>
>      g_assert(vrdl->granularity && is_power_of_2(vrdl->granularity));
> -    g_assert(vrdl->granularity >= 1 << ctz64(container->pgsizes));
> +    g_assert(container->pgsizes &&
> +             vrdl->granularity >= 1ULL << ctz64(container->pgsizes));
>
>      ram_discard_listener_init(&vrdl->listener,
>                                vfio_ram_discard_notify_populate,

> Fixes: d5015b801340 ("softmmu/memory: Pass ram_flags to
> qemu_ram_alloc_from_fd()")
>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/remote/memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/remote/memory.c b/hw/remote/memory.c
> index 472ed2a272..6e21ab1a45 100644
> --- a/hw/remote/memory.c
> +++ b/hw/remote/memory.c
> @@ -46,7 +46,7 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
>          subregion = g_new(MemoryRegion, 1);
>          memory_region_init_ram_from_fd(subregion, NULL,
>                                         name, sysmem_info->sizes[region],
> -                                       true, msg->fds[region],
> +                                       RAM_SHARED, msg->fds[region],
>                                         sysmem_info->offsets[region],
>                                         errp);
>

Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>