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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Shivaprasad G Bhat <sbhat@linux.ibm.com>
>Subject: [PATCH] vfio: container: Fix missing allocation of
>VFIOSpaprContainer
>
>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>

Reviewed-by: Zhenzhong Duan <Zhenzhong.duan@intel.com>

An alternative way is to introduce a VFIOIOMMUClass::create or
VFIOIOMMUClass::get_container_size.
But that needs some refactor to vfio_connect_container().

Thanks
Zhenzhong

>---
> 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;
>