Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753371AbdHKRCq (ORCPT ); Fri, 11 Aug 2017 13:02:46 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:46816 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752723AbdHKRCo (ORCPT ); Fri, 11 Aug 2017 13:02:44 -0400 Date: Fri, 11 Aug 2017 19:02:36 +0200 (CEST) From: Sebastian Ott X-X-Sender: sebott@schleppi To: Joerg Roedel cc: Gerald Schaefer , iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Joerg Roedel Subject: Re: [PATCH v3] iommu/s390: Add support for iommu_device handling In-Reply-To: <20170811073118.GF30515@8bytes.org> References: <1502274354-643-1-git-send-email-joro@8bytes.org> <20170811073118.GF30515@8bytes.org> User-Agent: Alpine 2.20 (LFD 67 2015-01-07) Organization: =?ISO-8859-15?Q?=22IBM_Deutschland_Research_&_Development_GmbH_=2F_Vorsitzende_des_Aufsichtsrats=3A_Martina_Koederitz_Gesch=E4ftsf=FChrung=3A_Dirk_Wittkopp_Sitz_der_Gesellschaft=3A_B=F6blingen_=2F_Registergericht?= =?ISO-8859-15?Q?=3A_Amtsgericht_Stuttgart=2C_HRB_243294=22?= MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-TM-AS-GCONF: 00 x-cbid: 17081117-0040-0000-0000-000003CFDC5F X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17081117-0041-0000-0000-000025CF7A8E Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-11_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=3 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1708110265 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3419 Lines: 113 Hello Joerg, On Fri, 11 Aug 2017, Joerg Roedel wrote: > Hey Sebastian, > > On Thu, Aug 10, 2017 at 09:07:06PM +0200, Sebastian Ott wrote: > > With this patch pci hot-unplug fails with a use after free or refcounting > > issue - I'm currently trying to understand what's going on... > > Let me know if I can help with debugging the issue, do you have a > backtrace for me to look at? I would have send backtraces but everyone looked different: random mem corruptions, panic during unrelated allocations and stuff like that. ..but I found the bug, actually 2 bugs: * That patch embedded a struct iommu_device within struct zpci_dev but the iommu_device has a release function (via its class) - so when the release function gets called it frees memory that was never allocated. The fix is to not embedd struct iommu_device in zpci_dev (see below) * iommu_release_device must not release the struct device but the structure it is embedded in: struct iommu_device (I'll send a patch for that) With these fixed it works fine. Sebastian --- arch/s390/include/asm/pci.h | 2 +- drivers/iommu/s390-iommu.c | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h index 386df9a..de3129e 100644 --- a/arch/s390/include/asm/pci.h +++ b/arch/s390/include/asm/pci.h @@ -123,7 +123,7 @@ struct zpci_dev { unsigned long iommu_pages; unsigned int next_bit; - struct iommu_device iommu_dev; /* IOMMU core handle */ + struct iommu_device *iommu_dev; /* IOMMU core handle */ char res_name[16]; struct zpci_bar_struct bars[PCI_BAR_COUNT]; diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c index 85f3bc5..58a7414 100644 --- a/drivers/iommu/s390-iommu.c +++ b/drivers/iommu/s390-iommu.c @@ -174,7 +174,7 @@ static int s390_iommu_add_device(struct device *dev) return PTR_ERR(group); iommu_group_put(group); - iommu_device_link(&zdev->iommu_dev, dev); + iommu_device_link(zdev->iommu_dev, dev); return 0; } @@ -201,7 +201,7 @@ static void s390_iommu_remove_device(struct device *dev) s390_iommu_detach_device(domain, dev); } - iommu_device_unlink(&zdev->iommu_dev, dev); + iommu_device_unlink(zdev->iommu_dev, dev); iommu_group_remove_device(dev); } @@ -336,21 +336,25 @@ int zpci_init_iommu(struct zpci_dev *zdev) { int rc = 0; - rc = iommu_device_sysfs_add(&zdev->iommu_dev, NULL, NULL, + zdev->iommu_dev = kzalloc(sizeof(*zdev->iommu_dev), GFP_KERNEL); + if (!zdev->iommu_dev) + return -ENOMEM; + + rc = iommu_device_sysfs_add(zdev->iommu_dev, NULL, NULL, "s390-iommu.%08x", zdev->fid); if (rc) goto out_err; - iommu_device_set_ops(&zdev->iommu_dev, &s390_iommu_ops); + iommu_device_set_ops(zdev->iommu_dev, &s390_iommu_ops); - rc = iommu_device_register(&zdev->iommu_dev); + rc = iommu_device_register(zdev->iommu_dev); if (rc) goto out_sysfs; return 0; out_sysfs: - iommu_device_sysfs_remove(&zdev->iommu_dev); + iommu_device_sysfs_remove(zdev->iommu_dev); out_err: return rc; @@ -358,8 +362,8 @@ int zpci_init_iommu(struct zpci_dev *zdev) void zpci_destroy_iommu(struct zpci_dev *zdev) { - iommu_device_unregister(&zdev->iommu_dev); - iommu_device_sysfs_remove(&zdev->iommu_dev); + iommu_device_unregister(zdev->iommu_dev); + iommu_device_sysfs_remove(zdev->iommu_dev); } static struct iommu_ops s390_iommu_ops = { -- 2.5.5