[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Qemu/Xen: Fix early freeing MSIX MMIO memory re
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH] Qemu/Xen: Fix early freeing MSIX MMIO memory region |
Date: |
Mon, 12 Oct 2015 13:45:38 +0100 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
On Mon, 12 Oct 2015, Paolo Bonzini wrote:
> On 12/10/2015 13:09, Stefano Stabellini wrote:
> > On Sun, 11 Oct 2015, Lan Tianyu wrote:
> >> From: <address@hidden>>
> >>
> >> 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 <address@hidden>
> >
> > Looks good to me. Paolo?
>
> Also looks good to me. Thanks!
I'll add it to my tree.
> >> 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
> >>
>