[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pci: add checking whether the device is realize
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH] pci: add checking whether the device is realized before unlinking the capability |
Date: |
Wed, 25 Jun 2014 15:28:32 +0300 |
On Wed, 2014-06-25 at 15:03 +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 25, 2014 at 06:59:08PM +0900, SeokYeon Hwang wrote:
> > In case of the unrealized "pdev", memory can be illegally accessed and
> > corrupted.
> > Refer to device_unparent() in the commit
> > 5c21ce77d7e5643089ceec556c0408445d017f32.
Hi,
Thank you for submitting the patch.
Can you please send to the list how to reproduce the issue?
Is this a regression? Before the above commit did it work?
> >
> > Change-Id: Iacb195a092c86d4c677ad0404582af104b2251ae
> > Signed-off-by: SeokYeon Hwang <address@hidden>
>
> Another case of qemu mailing list dropping patches :(
> I will bounce it now so people can see the original
> message.
>
>
> > ---
> > hw/pci/pci.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 49eca95..bb7f0c5 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -2056,7 +2056,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t
> > cap_id,
> > /* Unlink capability from the pci config space. */
> > void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
> > {
> > - uint8_t prev, offset = pci_find_capability_list(pdev, cap_id, &prev);
> > + uint8_t prev, offset;
> > + /* Check whether the device is realized or not */
> > + if (!pdev->qdev.realized) {
The 'qdev' field and 'realize' property are private, you should use one
of QOM's 'property_get' methods.
I am also concerned about adding the realize check here,
it seems too "deep" in implementation, seeing this checks everywhere
doesn't seem a good idea.
I think we should first understand the root cause and try making this
change in a higher level.
I hope I helped,
Marcel
> > + return;
> > + }
> > + offset = pci_find_capability_list(pdev, cap_id, &prev);
> > if (!offset)
> > return;
> > pdev->config[prev] = pdev->config[offset + PCI_CAP_LIST_NEXT];
> > --
> > 1.9.1
>