Re: [PATCH] vfio: container: Fix missing allocation of VFIOSpaprContainer

[Cédric Le Goater]

Shivaprasad,

On 5/9/24 9:14 PM, Shivaprasad G Bhat wrote:
> The commit 6ad359ec29 "(vfio/spapr: Move prereg_listener into
> spapr container)" began to use the newly introduced VFIOSpaprContainer
> structure.
>
> After several refactors, today the container_of(container,
> VFIOSpaprContainer, ABC) is used when VFIOSpaprContainer is actually
> not allocated. On PPC64 systems, this dereference is leading to corruption
> showing up as glibc malloc assertion during guest start when using vfio.
>
> Patch adds the missing allocation while also making the structure movement
> to vfio common header file.
>
> Fixes: 6ad359ec29 "(vfio/spapr: Move prereg_listener into spapr container)"
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>

Could you please give vfio-9.1 a try ? Thanks,

C.

https://github.com/legoater/qemu/commits/vfio-9.1

> ---
>   hw/vfio/container.c           |    6 ++++--
>   hw/vfio/spapr.c               |    6 ------
>   include/hw/vfio/vfio-common.h |    6 ++++++
>   3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 77bdec276e..ecaf5786d9 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -539,6 +539,7 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as,
>   {
>       VFIOContainer *container;
>       VFIOContainerBase *bcontainer;
> +    VFIOSpaprContainer *scontainer;
>       int ret, fd;
>       VFIOAddressSpace *space;
>
> @@ -611,7 +612,8 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as,
>           goto close_fd_exit;
>       }
>
> -    container = g_malloc0(sizeof(*container));
> +    scontainer = g_malloc0(sizeof(*scontainer));
> +    container = &scontainer->container;
>       container->fd = fd;
>       bcontainer = &container->bcontainer;
>
> @@ -675,7 +677,7 @@ unregister_container_exit:
>       vfio_cpr_unregister_container(bcontainer);
>
>   free_container_exit:
> -    g_free(container);
> +    g_free(scontainer);
>
>   close_fd_exit:
>       close(fd);
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 0d949bb728..78d218b7e7 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -24,12 +24,6 @@
>   #include "qapi/error.h"
>   #include "trace.h"
>
> -typedef struct VFIOSpaprContainer {
> -    VFIOContainer container;
> -    MemoryListener prereg_listener;
> -    QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
> -} VFIOSpaprContainer;
> -
>   static bool vfio_prereg_listener_skipped_section(MemoryRegionSection *section)
>   {
>       if (memory_region_is_iommu(section->mr)) {
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index b9da6c08ef..010fa68ac6 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -82,6 +82,12 @@ typedef struct VFIOContainer {
>       QLIST_HEAD(, VFIOGroup) group_list;
>   } VFIOContainer;
>
> +typedef struct VFIOSpaprContainer {
> +    VFIOContainer container;
> +    MemoryListener prereg_listener;
> +    QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
> +} VFIOSpaprContainer;
> +
>   typedef struct VFIOHostDMAWindow {
>       hwaddr min_iova;
>       hwaddr max_iova;