From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48969) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzmJr-0008N8-RK for qemu-devel@nongnu.org; Wed, 25 Jun 2014 08:29:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzmJm-0002Xg-Dd for qemu-devel@nongnu.org; Wed, 25 Jun 2014 08:28:55 -0400 Received: from mail-we0-x234.google.com ([2a00:1450:400c:c03::234]:51753) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzmJl-0002XQ-Ug for qemu-devel@nongnu.org; Wed, 25 Jun 2014 08:28:50 -0400 Received: by mail-we0-f180.google.com with SMTP id x48so1949348wes.11 for ; Wed, 25 Jun 2014 05:28:46 -0700 (PDT) Message-ID: <1403699312.20031.46.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Wed, 25 Jun 2014 15:28:32 +0300 In-Reply-To: <20140625120330.GA13856@redhat.com> References: <1403690348-19267-1-git-send-email-syeon.hwang@samsung.com> <20140625120330.GA13856@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] pci: add checking whether the device is realized before unlinking the capability Reply-To: marcel.a@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: SeokYeon Hwang Cc: "qemu-devel@nongnu.org" , "Michael S. Tsirkin" 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 > > 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 >