From 57d72e159b60456c8bb281736c02ddd3164037aa Mon Sep 17 00:00:00 2001 From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> Date: Thu, 14 Dec 2017 11:03:01 +0000 Subject: iommu/arm-smmu-v3: Don't free page table ops twice Kasan reports a double free when finalise_stage_fn fails: the io_pgtable ops are freed by arm_smmu_domain_finalise and then again by arm_smmu_domain_free. Prevent this by leaving pgtbl_ops empty on failure. Cc: <stable@vger.kernel.org> Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices") Reviewed-by: Robin Murphy <robin.murphy@arm.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> Signed-off-by: Will Deacon <will.deacon@arm.com> --- drivers/iommu/arm-smmu-v3.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/iommu/arm-smmu-v3.c') diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index f122071688fd..db4281d0e269 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1698,13 +1698,15 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain) domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap; domain->geometry.aperture_end = (1UL << ias) - 1; domain->geometry.force_aperture = true; - smmu_domain->pgtbl_ops = pgtbl_ops; ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg); - if (ret < 0) + if (ret < 0) { free_io_pgtable_ops(pgtbl_ops); + return ret; + } - return ret; + smmu_domain->pgtbl_ops = pgtbl_ops; + return 0; } static __le64 *arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid) -- cgit v1.2.3 From 563b5cbe334e9503ab2b234e279d500fc4f76018 Mon Sep 17 00:00:00 2001 From: Robin Murphy <robin.murphy@arm.com> Date: Tue, 2 Jan 2018 12:33:14 +0000 Subject: iommu/arm-smmu-v3: Cope with duplicated Stream IDs For PCI devices behind an aliasing PCIe-to-PCI/X bridge, the bridge alias to DevFn 0.0 on the subordinate bus may match the original RID of the device, resulting in the same SID being present in the device's fwspec twice. This causes trouble later in arm_smmu_write_strtab_ent() when we wind up visiting the STE a second time and find it already live. Avoid the issue by giving arm_smmu_install_ste_for_dev() the cleverness to skip over duplicates. It seems mildly counterintuitive compared to preventing the duplicates from existing in the first place, but since the DT and ACPI probe paths build their fwspecs differently, this is actually the cleanest and most self-contained way to deal with it. Cc: <stable@vger.kernel.org> Fixes: 8f78515425da ("iommu/arm-smmu: Implement of_xlate() for SMMUv3") Reported-by: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com> Tested-by: Tomasz Nowicki <Tomasz.Nowicki@cavium.com> Tested-by: Jayachandran C. <jnair@caviumnetworks.com> Signed-off-by: Robin Murphy <robin.murphy@arm.com> Signed-off-by: Will Deacon <will.deacon@arm.com> --- drivers/iommu/arm-smmu-v3.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/iommu/arm-smmu-v3.c') diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index db4281d0e269..744592d330ca 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1733,7 +1733,7 @@ static __le64 *arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid) static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec) { - int i; + int i, j; struct arm_smmu_master_data *master = fwspec->iommu_priv; struct arm_smmu_device *smmu = master->smmu; @@ -1741,6 +1741,13 @@ static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec) u32 sid = fwspec->ids[i]; __le64 *step = arm_smmu_get_step_for_sid(smmu, sid); + /* Bridged PCI devices may end up with duplicated IDs */ + for (j = 0; j < i; j++) + if (fwspec->ids[j] == sid) + break; + if (j < i) + continue; + arm_smmu_write_strtab_ent(smmu, sid, step, &master->ste); } } -- cgit v1.2.3 From b0c560f7d8a4b333bcc18f692d0af0d5cca90fe2 Mon Sep 17 00:00:00 2001 From: Robin Murphy <robin.murphy@arm.com> Date: Tue, 9 Jan 2018 16:17:27 +0000 Subject: iommu: Clean up of_iommu_init_fn Now that no more drivers rely on arbitrary early initialisation via an of_iommu_init_fn hook, let's clean up the redundant remnants. The IOMMU_OF_DECLARE() macro needs to remain for now, as the probe-deferral mechanism has no other nice way to detect built-in drivers before they have registered themselves, such that it can make the right decision. Reviewed-by: Sricharan R <sricharan@codeaurora.org> Signed-off-by: Robin Murphy <robin.murphy@arm.com> Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/arm-smmu-v3.c | 2 +- drivers/iommu/arm-smmu.c | 12 ++++++------ drivers/iommu/exynos-iommu.c | 2 +- drivers/iommu/ipmmu-vmsa.c | 4 ++-- drivers/iommu/msm_iommu.c | 2 +- drivers/iommu/of_iommu.c | 16 ---------------- drivers/iommu/qcom_iommu.c | 2 +- include/linux/of_iommu.h | 5 +---- 8 files changed, 13 insertions(+), 32 deletions(-) (limited to 'drivers/iommu/arm-smmu-v3.c') diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index f122071688fd..7f186beaa1a6 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2962,7 +2962,7 @@ static struct platform_driver arm_smmu_driver = { }; module_platform_driver(arm_smmu_driver); -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", NULL); +IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3"); MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations"); MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>"); diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 78d4c6b8f1ba..69e7c60792a8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -2211,12 +2211,12 @@ static struct platform_driver arm_smmu_driver = { }; module_platform_driver(arm_smmu_driver); -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", NULL); -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", NULL); -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", NULL); -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", NULL); -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", NULL); -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", NULL); +IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1"); +IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2"); +IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400"); +IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401"); +IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500"); +IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2"); MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations"); MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>"); diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 79c45650f8de..1c7f926fad0e 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1394,4 +1394,4 @@ err_reg_driver: } core_initcall(exynos_iommu_init); -IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu", NULL); +IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu"); diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c index 331dad909301..40ae6e87cb88 100644 --- a/drivers/iommu/ipmmu-vmsa.c +++ b/drivers/iommu/ipmmu-vmsa.c @@ -1108,8 +1108,8 @@ static void __exit ipmmu_exit(void) subsys_initcall(ipmmu_init); module_exit(ipmmu_exit); -IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa", NULL); -IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795", NULL); +IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa"); +IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795"); MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU"); MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>"); diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c index dda1ce87a070..0d3350463a3f 100644 --- a/drivers/iommu/msm_iommu.c +++ b/drivers/iommu/msm_iommu.c @@ -877,7 +877,7 @@ static void __exit msm_iommu_driver_exit(void) subsys_initcall(msm_iommu_driver_init); module_exit(msm_iommu_driver_exit); -IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu", NULL); +IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu"); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Stepan Moskovchenko <stepanm@codeaurora.org>"); diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 50947ebb6d17..5c36a8b7656a 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -231,19 +231,3 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, return ops; } - -static int __init of_iommu_init(void) -{ - struct device_node *np; - const struct of_device_id *match, *matches = &__iommu_of_table; - - for_each_matching_node_and_match(np, matches, &match) { - const of_iommu_init_fn init_fn = match->data; - - if (init_fn && init_fn(np)) - pr_err("Failed to initialise IOMMU %pOF\n", np); - } - - return 0; -} -postcore_initcall_sync(of_iommu_init); diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c index e07f02d00c68..65b9c99707f8 100644 --- a/drivers/iommu/qcom_iommu.c +++ b/drivers/iommu/qcom_iommu.c @@ -947,7 +947,7 @@ static void __exit qcom_iommu_exit(void) module_init(qcom_iommu_init); module_exit(qcom_iommu_exit); -IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1", NULL); +IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1"); MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations"); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h index cddfaff4d0b7..4fa654e4b5a9 100644 --- a/include/linux/of_iommu.h +++ b/include/linux/of_iommu.h @@ -34,9 +34,6 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev, extern struct of_device_id __iommu_of_table; -typedef int (*of_iommu_init_fn)(struct device_node *); - -#define IOMMU_OF_DECLARE(name, compat, fn) \ - _OF_DECLARE(iommu, name, compat, fn, of_iommu_init_fn) +#define IOMMU_OF_DECLARE(name, compat) OF_DECLARE_1(iommu, name, compat, NULL) #endif /* __OF_IOMMU_H */ -- cgit v1.2.3