Re: [Xen-devel] [PATCH] Qemu/Xen: Fix early freeing MSIX MMIO memory region

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Sun, 11 Oct 2015, Lan Tianyu wrote:
> From: <tianyu.lan@xxxxxxxxx>>
> 
> msix->mmio is added to XenPCIPassthroughState's object as property.
> object_finalize_child_property is called for XenPCIPassthroughState's
> object, which calls object_property_del_all, which is going to try to
> delete msix->mmio. object_finalize_child_property() will access
> msix->mmio's obj. But the whole msix struct has already been freed
> by xen_pt_msix_delete. This will cause segment fault when msix->mmio
> has been overwritten.
> 
> This patch is to fix the issue.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>

Looks good to me. Paolo?


>  hw/xen/xen_pt.c             |    8 ++++++++
>  hw/xen/xen_pt.h             |    1 +
>  hw/xen/xen_pt_config_init.c |    2 +-
>  hw/xen/xen_pt_msi.c         |   13 ++++++++++++-
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index 2b54f52..aa96288 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -938,10 +938,18 @@ static void xen_pci_passthrough_class_init(ObjectClass 
> *klass, void *data)
>      dc->props = xen_pci_passthrough_properties;
>  };
>  
> +static void xen_pci_passthrough_finalize(Object *obj)
> +{
> +    XenPCIPassthroughState *s = XEN_PT_DEVICE(obj);
> +
> +    xen_pt_msix_delete(s);
> +}
> +
>  static const TypeInfo xen_pci_passthrough_info = {
>      .name = TYPE_XEN_PT_DEVICE,
>      .parent = TYPE_PCI_DEVICE,
>      .instance_size = sizeof(XenPCIPassthroughState),
> +    .instance_finalize = xen_pci_passthrough_finalize,
>      .class_init = xen_pci_passthrough_class_init,
>  };
>  
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 3bc22eb..c545280 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -305,6 +305,7 @@ void xen_pt_msi_disable(XenPCIPassthroughState *s);
>  
>  int xen_pt_msix_init(XenPCIPassthroughState *s, uint32_t base);
>  void xen_pt_msix_delete(XenPCIPassthroughState *s);
> +void xen_pt_msix_unmap(XenPCIPassthroughState *s);
>  int xen_pt_msix_update(XenPCIPassthroughState *s);
>  int xen_pt_msix_update_remap(XenPCIPassthroughState *s, int bar_index);
>  void xen_pt_msix_disable(XenPCIPassthroughState *s);
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index 4a5bc11..0efee11 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -2079,7 +2079,7 @@ void xen_pt_config_delete(XenPCIPassthroughState *s)
>  
>      /* free MSI/MSI-X info table */
>      if (s->msix) {
> -        xen_pt_msix_delete(s);
> +        xen_pt_msix_unmap(s);
>      }
>      g_free(s->msi);
>  
> diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
> index e3d7194..82de2bc 100644
> --- a/hw/xen/xen_pt_msi.c
> +++ b/hw/xen/xen_pt_msi.c
> @@ -610,7 +610,7 @@ error_out:
>      return rc;
>  }
>  
> -void xen_pt_msix_delete(XenPCIPassthroughState *s)
> +void xen_pt_msix_unmap(XenPCIPassthroughState *s)
>  {
>      XenPTMSIX *msix = s->msix;
>  
> @@ -627,6 +627,17 @@ void xen_pt_msix_delete(XenPCIPassthroughState *s)
>      }
>  
>      memory_region_del_subregion(&s->bar[msix->bar_index], &msix->mmio);
> +}
> +
> +void xen_pt_msix_delete(XenPCIPassthroughState *s)
> +{
> +    XenPTMSIX *msix = s->msix;
> +
> +    if (!msix) {
> +        return;
> +    }
> +
> +    object_unparent(OBJECT(&msix->mmio));
>  
>      g_free(s->msix);
>      s->msix = NULL;
> -- 
> 1.7.9.5
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel