[PATCH] iommu/mediatek: check return value after calling platform_get_resource()
It will cause null-ptr-deref if platform_get_resource() returns NULL, we need check the return value. Fixes: 42d57fc58aeb ("iommu/mediatek: Initialise/Remove for multi bank dev") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/mtk_iommu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index b2ae84046249..b45b0c1cfff9 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -1174,6 +1174,8 @@ static int mtk_iommu_probe(struct platform_device *pdev) banks_num = data->plat_data->banks_num; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; if (resource_size(res) < banks_num * MTK_IOMMU_BANK_SZ) { dev_err(dev, "banknr %d. res %pR is not enough.\n", banks_num, res); return -EINVAL; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie()
cookie_alloc() is called by iommu_get_dma_cookie() and iommu_get_msi_cookie(), but the mutex is only initialized in iommu_get_dma_cookie(), move mutex_init() into cookie_alloc() to make sure the mutex will be initialized. Fixes: ac9a5d522bb8 ("iommu/dma: Fix race condition during iova_domain initialization") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/dma-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 1910f4f1612b..e29157380c48 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -294,6 +294,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type) if (cookie) { INIT_LIST_HEAD(&cookie->msi_page_list); cookie->type = type; + mutex_init(&cookie->mutex); } return cookie; } @@ -311,7 +312,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain) if (!domain->iova_cookie) return -ENOMEM; - mutex_init(&domain->iova_cookie->mutex); return 0; } -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/arm-smmu-v3: check return value after calling platform_get_resource()
It will cause null-ptr-deref if platform_get_resource() returns NULL, we need check the return value. Signed-off-by: Yang Yingliang --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 627a3ed5ee8f..88817a3376ef 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3770,6 +3770,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev) /* Base address */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; if (resource_size(res) < arm_smmu_resource_size(smmu)) { dev_err(dev, "MMIO region too small (%pr)\n", res); return -EINVAL; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/arm-smmu: fix possible null-ptr-deref in arm_smmu_device_probe()
It will cause null-ptr-deref when using 'res', if platform_get_resource() returns NULL, so move using 'res' after devm_ioremap_resource() that will check it to avoid null-ptr-deref. And use devm_platform_get_and_ioremap_resource() to simplify code. Fixes: 9648cbc9625b ("iommu/arm-smmu: Make use of the iommu_register interface") Signed-off-by: Yang Yingliang --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 568cce590ccc..52b71f6aee3f 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -2092,11 +2092,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev) if (err) return err; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - ioaddr = res->start; - smmu->base = devm_ioremap_resource(dev, res); + smmu->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(smmu->base)) return PTR_ERR(smmu->base); + ioaddr = res->start; /* * The resource size should effectively match the value of SMMU_TOP; * stash that temporarily until we know PAGESIZE to validate it with. -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/dart: check return value after calling platform_get_resource()
It will cause null-ptr-deref in resource_size(), if platform_get_resource() returns NULL, move calling resource_size() after devm_ioremap_resource() that will check 'res' to avoid null-ptr-deref. And use devm_platform_get_and_ioremap_resource() to simplify code. Fixes: 46d1fb072e76 ("iommu/dart: Add DART iommu driver") Signed-off-by: Yang Yingliang --- drivers/iommu/apple-dart.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c index decafb07ad08..15b77f16cfa3 100644 --- a/drivers/iommu/apple-dart.c +++ b/drivers/iommu/apple-dart.c @@ -859,16 +859,15 @@ static int apple_dart_probe(struct platform_device *pdev) dart->dev = dev; spin_lock_init(&dart->lock); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + dart->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res); + if (IS_ERR(dart->regs)) + return PTR_ERR(dart->regs); + if (resource_size(res) < 0x4000) { dev_err(dev, "MMIO region too small (%pr)\n", res); return -EINVAL; } - dart->regs = devm_ioremap_resource(dev, res); - if (IS_ERR(dart->regs)) - return PTR_ERR(dart->regs); - dart->irq = platform_get_irq(pdev, 0); if (dart->irq < 0) return -ENODEV; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH -next] iommu/arm-smmu: Fix missing unlock on error in arm_smmu_device_group()
Add the missing unlock before return from function arm_smmu_device_group() in the error handling case. Fixes: b1a1347912a7 ("iommu/arm-smmu: Fix race condition during iommu_group creation") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 73893180ec7e..4bc75c4ce402 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1478,8 +1478,10 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev) mutex_lock(&smmu->stream_map_mutex); for_each_cfg_sme(cfg, fwspec, i, idx) { if (group && smmu->s2crs[idx].group && - group != smmu->s2crs[idx].group) + group != smmu->s2crs[idx].group) { + mutex_unlock(&smmu->stream_map_mutex); return ERR_PTR(-EINVAL); + } group = smmu->s2crs[idx].group; } -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu: return error code when it can't get group
It's better to return error code if it can't get group in iommu_probe_device(). It's no function change. Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index b53446bb8c6b..6f4a32df90f6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -253,8 +253,10 @@ int iommu_probe_device(struct device *dev) goto err_out; group = iommu_group_get(dev); - if (!group) + if (!group) { + ret = -ENODEV; goto err_release; + } /* * Try to allocate a default domain - needs support from the -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: fix return error code in iommu_probe_device()
On 2020/11/25 19:35, Will Deacon wrote: On Wed, Nov 25, 2020 at 09:54:34AM +0800, Yang Yingliang wrote: On 2020/11/18 6:41, Will Deacon wrote: On Tue, Nov 17, 2020 at 07:11:28PM +0800, Yang Yingliang wrote: On 2020/11/17 17:40, Lu Baolu wrote: On 2020/11/17 10:52, Yang Yingliang wrote: If iommu_group_get() failed, it need return error code in iommu_probe_device(). Fixes: cf193888bfbd ("iommu: Move new probe_device path...") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index b53446bb8c6b..6f4a32df90f6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -253,8 +253,10 @@ int iommu_probe_device(struct device *dev) goto err_out; group = iommu_group_get(dev); - if (!group) + if (!group) { + ret = -ENODEV; Can you please explain why you use -ENODEV here? Before 79659190ee97 ("iommu: Don't take group reference in iommu_alloc_default_domain()"), in iommu_alloc_default_domain(), if group is NULL, it will return -ENODEV. Hmm. While I think the patch is ok, I'm not sure it qualifies as a fix. Has iommu_probe_device() ever propagated this error? The commit you identify in the 'Fixes:' tag doesn't seem to change this afaict. I think after this commit 439945e74a4b ("iommu: Move default domain allocation to iommu_probe_device()"), That SHA doesn't exist in my tree (maybe you mean 6e1aa2049154?). But even then, I'm not sure 6e1aa2049154 is actually broken if you look at the interaction with group creation in __iommu_probe_device(). In fact, isn't that the case in mainline too? If __iommu_probe_device() returns 0, then we _know_ a group exists and so iommu_group_get() will succeed. I'm still happy with the patch in case this changes in future, but it doesn't appear to be fixing anything. Do you agree? Yes, I look into the __iommu_probe_device(), if it can't get group, it will return error first. Do I need send a v2 without the fix tag ? Will . ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: fix return error code in iommu_probe_device()
On 2020/11/18 6:41, Will Deacon wrote: On Tue, Nov 17, 2020 at 07:11:28PM +0800, Yang Yingliang wrote: On 2020/11/17 17:40, Lu Baolu wrote: On 2020/11/17 10:52, Yang Yingliang wrote: If iommu_group_get() failed, it need return error code in iommu_probe_device(). Fixes: cf193888bfbd ("iommu: Move new probe_device path...") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index b53446bb8c6b..6f4a32df90f6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -253,8 +253,10 @@ int iommu_probe_device(struct device *dev) goto err_out; group = iommu_group_get(dev); - if (!group) + if (!group) { + ret = -ENODEV; Can you please explain why you use -ENODEV here? Before 79659190ee97 ("iommu: Don't take group reference in iommu_alloc_default_domain()"), in iommu_alloc_default_domain(), if group is NULL, it will return -ENODEV. Hmm. While I think the patch is ok, I'm not sure it qualifies as a fix. Has iommu_probe_device() ever propagated this error? The commit you identify in the 'Fixes:' tag doesn't seem to change this afaict. I think after this commit 439945e74a4b ("iommu: Move default domain allocation to iommu_probe_device()"), iommu_probe_device() won't return error code if group is NULL. I can add this fix tag in v2. Will . ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: fix return error code in iommu_probe_device()
On 2020/11/17 17:40, Lu Baolu wrote: Hi Yingliang, On 2020/11/17 10:52, Yang Yingliang wrote: If iommu_group_get() failed, it need return error code in iommu_probe_device(). Fixes: cf193888bfbd ("iommu: Move new probe_device path...") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index b53446bb8c6b..6f4a32df90f6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -253,8 +253,10 @@ int iommu_probe_device(struct device *dev) goto err_out; group = iommu_group_get(dev); - if (!group) + if (!group) { + ret = -ENODEV; Can you please explain why you use -ENODEV here? Before 79659190ee97 ("iommu: Don't take group reference in iommu_alloc_default_domain()"), in iommu_alloc_default_domain(), if group is NULL, it will return -ENODEV. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/iommu.c?h=v5.10-rc4&id=70b8170e55d3ca9503a53211967faee6b5f18b19 Best regards, baolu goto err_release; + } /* * Try to allocate a default domain - needs support from the . ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: fix return error code in iommu_probe_device()
If iommu_group_get() failed, it need return error code in iommu_probe_device(). Fixes: cf193888bfbd ("iommu: Move new probe_device path...") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang --- drivers/iommu/iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index b53446bb8c6b..6f4a32df90f6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -253,8 +253,10 @@ int iommu_probe_device(struct device *dev) goto err_out; group = iommu_group_get(dev); - if (!group) + if (!group) { + ret = -ENODEV; goto err_release; + } /* * Try to allocate a default domain - needs support from the -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu