| Message ID | 20200429133712.31431-1-joro@8bytes.org |
|---|---|
| Headers | show |
| Series | iommu: Move iommu_group setup to IOMMU core code | expand |
On 2020/4/29 21:36, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > Convert the Intel IOMMU driver to use the probe_device() and > release_device() call-backs of iommu_ops, so that the iommu core code > does the group and sysfs setup. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> Best regards, baolu > --- > drivers/iommu/intel-iommu.c | 67 ++++--------------------------------- > 1 file changed, 6 insertions(+), 61 deletions(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index b9f905a55dda..b906727f5b85 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -5781,78 +5781,27 @@ static bool intel_iommu_capable(enum iommu_cap cap) > return false; > } > > -static int intel_iommu_add_device(struct device *dev) > +static struct iommu_device *intel_iommu_probe_device(struct device *dev) > { > - struct dmar_domain *dmar_domain; > - struct iommu_domain *domain; > struct intel_iommu *iommu; > - struct iommu_group *group; > u8 bus, devfn; > - int ret; > > iommu = device_to_iommu(dev, &bus, &devfn); > if (!iommu) > - return -ENODEV; > - > - iommu_device_link(&iommu->iommu, dev); > + return ERR_PTR(-ENODEV); > > if (translation_pre_enabled(iommu)) > dev->archdata.iommu = DEFER_DEVICE_DOMAIN_INFO; > > - group = iommu_group_get_for_dev(dev); > - > - if (IS_ERR(group)) { > - ret = PTR_ERR(group); > - goto unlink; > - } > - > - iommu_group_put(group); > - > - domain = iommu_get_domain_for_dev(dev); > - dmar_domain = to_dmar_domain(domain); > - if (domain->type == IOMMU_DOMAIN_DMA) { > - if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) { > - ret = iommu_request_dm_for_dev(dev); > - if (ret) { > - dmar_remove_one_dev_info(dev); > - dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN; > - domain_add_dev_info(si_domain, dev); > - dev_info(dev, > - "Device uses a private identity domain.\n"); > - } > - } > - } else { > - if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) { > - ret = iommu_request_dma_domain_for_dev(dev); > - if (ret) { > - dmar_remove_one_dev_info(dev); > - dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN; > - if (!get_private_domain_for_dev(dev)) { > - dev_warn(dev, > - "Failed to get a private domain.\n"); > - ret = -ENOMEM; > - goto unlink; > - } > - > - dev_info(dev, > - "Device uses a private dma domain.\n"); > - } > - } > - } > - > if (device_needs_bounce(dev)) { > dev_info(dev, "Use Intel IOMMU bounce page dma_ops\n"); > set_dma_ops(dev, &bounce_dma_ops); > } > > - return 0; > - > -unlink: > - iommu_device_unlink(&iommu->iommu, dev); > - return ret; > + return &iommu->iommu; > } > > -static void intel_iommu_remove_device(struct device *dev) > +static void intel_iommu_release_device(struct device *dev) > { > struct intel_iommu *iommu; > u8 bus, devfn; > @@ -5863,10 +5812,6 @@ static void intel_iommu_remove_device(struct device *dev) > > dmar_remove_one_dev_info(dev); > > - iommu_group_remove_device(dev); > - > - iommu_device_unlink(&iommu->iommu, dev); > - > if (device_needs_bounce(dev)) > set_dma_ops(dev, NULL); > } > @@ -6198,8 +6143,8 @@ const struct iommu_ops intel_iommu_ops = { > .map = intel_iommu_map, > .unmap = intel_iommu_unmap, > .iova_to_phys = intel_iommu_iova_to_phys, > - .add_device = intel_iommu_add_device, > - .remove_device = intel_iommu_remove_device, > + .probe_device = intel_iommu_probe_device, > + .release_device = intel_iommu_release_device, > .get_resv_regions = intel_iommu_get_resv_regions, > .put_resv_regions = generic_iommu_put_resv_regions, > .apply_resv_region = intel_iommu_apply_resv_region, >
On Wed, Apr 29, 2020 at 03:36:38PM +0200, Joerg Roedel wrote: > Please review. If there are no objections I plan to put these patches > into the IOMMU tree early next week. Series is now applied.
On 2020-07-01 01:40, Qian Cai wrote: > Looks like this patchset introduced an use-after-free on arm-smmu-v3. > > Reproduced using mlx5, > > # echo 1 > /sys/class/net/enp11s0f1np1/device/sriov_numvfs > # echo 0 > /sys/class/net/enp11s0f1np1/device/sriov_numvfs > > The .config, > https://github.com/cailca/linux-mm/blob/master/arm64.config > > Looking at the free stack, > > iommu_release_device->iommu_group_remove_device > > was introduced in 07/34 ("iommu: Add probe_device() and release_device() > call-backs"). Right, iommu_group_remove_device can tear down the group and call ->domain_free before the driver has any knowledge of the last device going away via the ->release_device call. I guess the question is do we simply flip the call order in iommu_release_device() so drivers can easily clean up their internal per-device state first, or do we now want them to be robust against freeing domains with devices still nominally attached? Robin.
On Fri, Jul 03, 2020 at 08:17:09PM -0400, Qian Cai wrote: > FYI, I have just sent a patch to fix this, > > https://lore.kernel.org/linux-iommu/20200704001003.2303-1-cai@lca.pw/ Just queued that fix, thanks. Please don't send patches to my suse email address, use only the 8bytes.org one. Thanks, Joerg
Hi, here is the third version of this patch-set. Older versions can be found here: v1: https://lore.kernel.org/lkml/20200407183742.4344-1-joro@8bytes.org/ (Has some more introductory text) v2: https://lore.kernel.org/lkml/20200414131542.25608-1-joro@8bytes.org/ Changes v2 -> v3: * Rebased v5.7-rc3 * Added a missing iommu_group_put() as reported by Lu Baolu. * Added a patch to consolidate more initialization work in __iommu_probe_device(), fixing a bug where no 'struct device_iommu' was allocated in the hotplug path. There is also a git-branch available with these patches applied: https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v3 Please review. If there are no objections I plan to put these patches into the IOMMU tree early next week. Thanks, Joerg Joerg Roedel (33): iommu: Move default domain allocation to separate function iommu/amd: Implement iommu_ops->def_domain_type call-back iommu/vt-d: Wire up iommu_ops->def_domain_type iommu/amd: Remove dma_mask check from check_device() iommu/amd: Return -ENODEV in add_device when device is not handled by IOMMU iommu: Add probe_device() and release_device() call-backs iommu: Move default domain allocation to iommu_probe_device() iommu: Keep a list of allocated groups in __iommu_probe_device() iommu: Move new probe_device path to separate function iommu: Split off default domain allocation from group assignment iommu: Move iommu_group_create_direct_mappings() out of iommu_group_add_device() iommu: Export bus_iommu_probe() and make is safe for re-probing iommu/amd: Remove dev_data->passthrough iommu/amd: Convert to probe/release_device() call-backs iommu/vt-d: Convert to probe/release_device() call-backs iommu/arm-smmu: Convert to probe/release_device() call-backs iommu/pamu: Convert to probe/release_device() call-backs iommu/s390: Convert to probe/release_device() call-backs iommu/virtio: Convert to probe/release_device() call-backs iommu/msm: Convert to probe/release_device() call-backs iommu/mediatek: Convert to probe/release_device() call-backs iommu/mediatek-v1 Convert to probe/release_device() call-backs iommu/qcom: Convert to probe/release_device() call-backs iommu/rockchip: Convert to probe/release_device() call-backs iommu/tegra: Convert to probe/release_device() call-backs iommu/renesas: Convert to probe/release_device() call-backs iommu/omap: Remove orphan_dev tracking iommu/omap: Convert to probe/release_device() call-backs iommu/exynos: Use first SYSMMU in controllers list for IOMMU core iommu/exynos: Convert to probe/release_device() call-backs iommu: Remove add_device()/remove_device() code-paths iommu: Move more initialization to __iommu_probe_device() iommu: Unexport iommu_group_get_for_dev() Sai Praneeth Prakhya (1): iommu: Add def_domain_type() callback in iommu_ops drivers/iommu/amd_iommu.c | 97 ++++---- drivers/iommu/amd_iommu_types.h | 1 - drivers/iommu/arm-smmu-v3.c | 38 +--- drivers/iommu/arm-smmu.c | 39 ++-- drivers/iommu/exynos-iommu.c | 24 +- drivers/iommu/fsl_pamu_domain.c | 22 +- drivers/iommu/intel-iommu.c | 68 +----- drivers/iommu/iommu.c | 387 +++++++++++++++++++++++++------- drivers/iommu/ipmmu-vmsa.c | 60 ++--- drivers/iommu/msm_iommu.c | 34 +-- drivers/iommu/mtk_iommu.c | 24 +- drivers/iommu/mtk_iommu_v1.c | 50 ++--- drivers/iommu/omap-iommu.c | 99 ++------ drivers/iommu/qcom_iommu.c | 24 +- drivers/iommu/rockchip-iommu.c | 26 +-- drivers/iommu/s390-iommu.c | 22 +- drivers/iommu/tegra-gart.c | 24 +- drivers/iommu/tegra-smmu.c | 31 +-- drivers/iommu/virtio-iommu.c | 41 +--- include/linux/iommu.h | 21 +- 20 files changed, 531 insertions(+), 601 deletions(-)