FAILED: patch "[PATCH] iommu/qcom: fix device leak on of_xlate()" failed to apply to 6.1-stable tree
The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x 6a3908ce56e6879920b44ef136252b2f0c954194 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2026010519-padlock-footman-35a7@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 6a3908ce56e6879920b44ef136252b2f0c954194 Mon Sep 17 00:00:00 2001 From: Johan Hovold johan@kernel.org Date: Mon, 20 Oct 2025 06:53:06 +0200 Subject: [PATCH] iommu/qcom: fix device leak on of_xlate()
Make sure to drop the reference taken to the iommu platform device when looking up its driver data during of_xlate().
Note that commit e2eae09939a8 ("iommu/qcom: add missing put_device() call in qcom_iommu_of_xlate()") fixed the leak in a couple of error paths, but the reference is still leaking on success and late failures.
Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu") Cc: stable@vger.kernel.org # 4.14: e2eae09939a8 Cc: Rob Clark robin.clark@oss.qualcomm.com Cc: Yu Kuai yukuai3@huawei.com Acked-by: Robin Murphy robin.murphy@arm.com Signed-off-by: Johan Hovold johan@kernel.org Signed-off-by: Joerg Roedel joerg.roedel@amd.com
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 9222a4a48bb3..f69d9276dc55 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -566,14 +566,14 @@ static int qcom_iommu_of_xlate(struct device *dev,
qcom_iommu = platform_get_drvdata(iommu_pdev);
+ put_device(&iommu_pdev->dev); + /* make sure the asid specified in dt is valid, so we don't have * to sanity check this elsewhere: */ if (WARN_ON(asid > qcom_iommu->max_asid) || - WARN_ON(qcom_iommu->ctxs[asid] == NULL)) { - put_device(&iommu_pdev->dev); + WARN_ON(qcom_iommu->ctxs[asid] == NULL)) return -EINVAL; - }
if (!dev_iommu_priv_get(dev)) { dev_iommu_priv_set(dev, qcom_iommu); @@ -582,10 +582,8 @@ static int qcom_iommu_of_xlate(struct device *dev, * multiple different iommu devices. Multiple context * banks are ok, but multiple devices are not: */ - if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) { - put_device(&iommu_pdev->dev); + if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) return -EINVAL; - } }
return iommu_fwspec_add_ids(dev, &asid, 1);
From: Uwe Kleine-König u.kleine-koenig@pengutronix.de
[ Upstream commit a2972cb89935160bfe515b15d28a77694723ac06 ]
The remove and shutdown callback are only called after probe completed successfully. In this case platform_set_drvdata() was called with a non-NULL argument and so smmu is never NULL. Other functions in this driver also don't check for smmu being non-NULL before using it.
Also note that returning an error code from a remove callback doesn't result in the device staying bound. It's still removed and devm allocated resources are freed (among others *smmu and the register mapping). So after an early exit to iommu device stayed around and using it probably oopses.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de Reviewed-by: Robin Murphy robin.murphy@arm.com Link: https://lore.kernel.org/r/20230321084125.337021-2-u.kleine-koenig@pengutroni... Signed-off-by: Joerg Roedel jroedel@suse.de Stable-dep-of: 6a3908ce56e6 ("iommu/qcom: fix device leak on of_xlate()") Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 294120049750..fcd2cfd12e7f 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -2209,9 +2209,6 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev) { struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
- if (!smmu) - return; - if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS)) dev_notice(&pdev->dev, "disabling translation\n");
@@ -2232,9 +2229,6 @@ static int arm_smmu_device_remove(struct platform_device *pdev) { struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
- if (!smmu) - return -ENODEV; - iommu_device_unregister(&smmu->iommu); iommu_device_sysfs_remove(&smmu->iommu);
From: Uwe Kleine-König u.kleine-koenig@pengutronix.de
[ Upstream commit 62565a77c2323d32f2be737455729ac7d3efe6ad ]
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is (mostly) ignored and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new() which already returns void.
Trivially convert this driver from always returning zero in the remove callback to the void returning variant.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de Link: https://lore.kernel.org/r/20230321084125.337021-5-u.kleine-koenig@pengutroni... Signed-off-by: Joerg Roedel jroedel@suse.de Stable-dep-of: 6a3908ce56e6 ("iommu/qcom: fix device leak on of_xlate()") Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 ++---- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 12 ++++-------- 2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index fcd2cfd12e7f..96a4ae3fc778 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -2225,7 +2225,7 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev) clk_bulk_unprepare(smmu->num_clks, smmu->clks); }
-static int arm_smmu_device_remove(struct platform_device *pdev) +static void arm_smmu_device_remove(struct platform_device *pdev) { struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
@@ -2233,8 +2233,6 @@ static int arm_smmu_device_remove(struct platform_device *pdev) iommu_device_sysfs_remove(&smmu->iommu);
arm_smmu_device_shutdown(pdev); - - return 0; }
static int __maybe_unused arm_smmu_runtime_resume(struct device *dev) @@ -2310,7 +2308,7 @@ static struct platform_driver arm_smmu_driver = { .suppress_bind_attrs = true, }, .probe = arm_smmu_device_probe, - .remove = arm_smmu_device_remove, + .remove_new = arm_smmu_device_remove, .shutdown = arm_smmu_device_shutdown, }; module_platform_driver(arm_smmu_driver); diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 5b9cb9fcc352..72cc66ffab67 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -715,7 +715,7 @@ static int qcom_iommu_ctx_probe(struct platform_device *pdev) return 0; }
-static int qcom_iommu_ctx_remove(struct platform_device *pdev) +static void qcom_iommu_ctx_remove(struct platform_device *pdev) { struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(pdev->dev.parent); struct qcom_iommu_ctx *ctx = platform_get_drvdata(pdev); @@ -723,8 +723,6 @@ static int qcom_iommu_ctx_remove(struct platform_device *pdev) platform_set_drvdata(pdev, NULL);
qcom_iommu->ctxs[ctx->asid - 1] = NULL; - - return 0; }
static const struct of_device_id ctx_of_match[] = { @@ -739,7 +737,7 @@ static struct platform_driver qcom_iommu_ctx_driver = { .of_match_table = ctx_of_match, }, .probe = qcom_iommu_ctx_probe, - .remove = qcom_iommu_ctx_remove, + .remove_new = qcom_iommu_ctx_remove, };
static bool qcom_iommu_has_secure_context(struct qcom_iommu_dev *qcom_iommu) @@ -857,7 +855,7 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) return ret; }
-static int qcom_iommu_device_remove(struct platform_device *pdev) +static void qcom_iommu_device_remove(struct platform_device *pdev) { struct qcom_iommu_dev *qcom_iommu = platform_get_drvdata(pdev);
@@ -865,8 +863,6 @@ static int qcom_iommu_device_remove(struct platform_device *pdev) platform_set_drvdata(pdev, NULL); iommu_device_sysfs_remove(&qcom_iommu->iommu); iommu_device_unregister(&qcom_iommu->iommu); - - return 0; }
static int __maybe_unused qcom_iommu_resume(struct device *dev) @@ -903,7 +899,7 @@ static struct platform_driver qcom_iommu_driver = { .pm = &qcom_iommu_pm_ops, }, .probe = qcom_iommu_device_probe, - .remove = qcom_iommu_device_remove, + .remove_new = qcom_iommu_device_remove, };
static int __init qcom_iommu_init(void)
From: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
[ Upstream commit fcf226f1f7083cba76af47bf8dd764b68b149cd2 ]
As specified in this driver, the context banks are 0x1000 apart but on some SoCs the context number does not necessarily match this logic, hence we end up using the wrong ASID: keeping in mind that this IOMMU implementation relies heavily on SCM (TZ) calls, it is mandatory that we communicate the right context number.
Since this is all about how context banks are mapped in firmware, which may be board dependent (as a different firmware version may eventually change the expected context bank numbers), introduce a new property "qcom,ctx-asid": when found, the ASID will be forced as read from the devicetree.
When "qcom,ctx-asid" is not found, this driver retains the previous behavior as to avoid breaking older devicetrees or systems that do not require forcing ASID numbers.
Signed-off-by: Marijn Suijten marijn.suijten@somainline.org [Marijn: Rebased over next-20221111] Signed-off-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com Reviewed-by: Konrad Dybcio konrad.dybcio@linaro.org Link: https://lore.kernel.org/r/20230622092742.74819-3-angelogioacchino.delregno@c... Signed-off-by: Will Deacon will@kernel.org Stable-dep-of: 6a3908ce56e6 ("iommu/qcom: fix device leak on of_xlate()") Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 72cc66ffab67..dd8d5e2f3c08 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -563,7 +563,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) * index into qcom_iommu->ctxs: */ if (WARN_ON(asid < 1) || - WARN_ON(asid > qcom_iommu->num_ctxs)) { + WARN_ON(asid > qcom_iommu->num_ctxs) || + WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { put_device(&iommu_pdev->dev); return -EINVAL; } @@ -650,7 +651,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev)
static int get_asid(const struct device_node *np) { - u32 reg; + u32 reg, val; + int asid;
/* read the "reg" property directly to get the relative address * of the context bank, and calculate the asid from that: @@ -658,7 +660,17 @@ static int get_asid(const struct device_node *np) if (of_property_read_u32_index(np, "reg", 0, ®)) return -ENODEV;
- return reg / 0x1000; /* context banks are 0x1000 apart */ + /* + * Context banks are 0x1000 apart but, in some cases, the ASID + * number doesn't match to this logic and needs to be passed + * from the DT configuration explicitly. + */ + if (!of_property_read_u32(np, "qcom,ctx-asid", &val)) + asid = val; + else + asid = reg / 0x1000; + + return asid; }
static int qcom_iommu_ctx_probe(struct platform_device *pdev)
From: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
[ Upstream commit ec5601661bfcdc206e6ceba1b97837e763dab1ba ]
This driver was indexing the contexts by asid-1, which is probably done under the assumption that the first ASID is always 1. Unfortunately this is not always true: at least for MSM8956 and MSM8976's GPU IOMMU, the gpu_user context's ASID number is zero. To allow using a zero asid number, index the contexts by `asid` instead of by `asid - 1`.
While at it, also enhance human readability by renaming the `num_ctxs` member of struct qcom_iommu_dev to `max_asid`.
Signed-off-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com Reviewed-by: Konrad Dybcio konrad.dybcio@linaro.org Link: https://lore.kernel.org/r/20230622092742.74819-5-angelogioacchino.delregno@c... Signed-off-by: Will Deacon will@kernel.org Stable-dep-of: 6a3908ce56e6 ("iommu/qcom: fix device leak on of_xlate()") Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index dd8d5e2f3c08..0c27de3fd2f6 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -51,8 +51,8 @@ struct qcom_iommu_dev { struct clk_bulk_data clks[CLK_NUM]; void __iomem *local_base; u32 sec_id; - u8 num_ctxs; - struct qcom_iommu_ctx *ctxs[]; /* indexed by asid-1 */ + u8 max_asid; + struct qcom_iommu_ctx *ctxs[]; /* indexed by asid */ };
struct qcom_iommu_ctx { @@ -94,7 +94,7 @@ static struct qcom_iommu_ctx * to_ctx(struct qcom_iommu_domain *d, unsigned asid struct qcom_iommu_dev *qcom_iommu = d->iommu; if (!qcom_iommu) return NULL; - return qcom_iommu->ctxs[asid - 1]; + return qcom_iommu->ctxs[asid]; }
static inline void @@ -559,12 +559,10 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) qcom_iommu = platform_get_drvdata(iommu_pdev);
/* make sure the asid specified in dt is valid, so we don't have - * to sanity check this elsewhere, since 'asid - 1' is used to - * index into qcom_iommu->ctxs: + * to sanity check this elsewhere: */ - if (WARN_ON(asid < 1) || - WARN_ON(asid > qcom_iommu->num_ctxs) || - WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { + if (WARN_ON(asid > qcom_iommu->max_asid) || + WARN_ON(qcom_iommu->ctxs[asid] == NULL)) { put_device(&iommu_pdev->dev); return -EINVAL; } @@ -722,7 +720,7 @@ static int qcom_iommu_ctx_probe(struct platform_device *pdev)
dev_dbg(dev, "found asid %u\n", ctx->asid);
- qcom_iommu->ctxs[ctx->asid - 1] = ctx; + qcom_iommu->ctxs[ctx->asid] = ctx;
return 0; } @@ -734,7 +732,7 @@ static void qcom_iommu_ctx_remove(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);
- qcom_iommu->ctxs[ctx->asid - 1] = NULL; + qcom_iommu->ctxs[ctx->asid] = NULL; }
static const struct of_device_id ctx_of_match[] = { @@ -781,11 +779,11 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) for_each_child_of_node(dev->of_node, child) max_asid = max(max_asid, get_asid(child));
- qcom_iommu = devm_kzalloc(dev, struct_size(qcom_iommu, ctxs, max_asid), + qcom_iommu = devm_kzalloc(dev, struct_size(qcom_iommu, ctxs, max_asid + 1), GFP_KERNEL); if (!qcom_iommu) return -ENOMEM; - qcom_iommu->num_ctxs = max_asid; + qcom_iommu->max_asid = max_asid; qcom_iommu->dev = dev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
From: Johan Hovold johan@kernel.org
[ Upstream commit 6a3908ce56e6879920b44ef136252b2f0c954194 ]
Make sure to drop the reference taken to the iommu platform device when looking up its driver data during of_xlate().
Note that commit e2eae09939a8 ("iommu/qcom: add missing put_device() call in qcom_iommu_of_xlate()") fixed the leak in a couple of error paths, but the reference is still leaking on success and late failures.
Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu") Cc: stable@vger.kernel.org # 4.14: e2eae09939a8 Cc: Rob Clark robin.clark@oss.qualcomm.com Cc: Yu Kuai yukuai3@huawei.com Acked-by: Robin Murphy robin.murphy@arm.com Signed-off-by: Johan Hovold johan@kernel.org Signed-off-by: Joerg Roedel joerg.roedel@amd.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 0c27de3fd2f6..3d4428e688ee 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -558,14 +558,14 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
qcom_iommu = platform_get_drvdata(iommu_pdev);
+ put_device(&iommu_pdev->dev); + /* make sure the asid specified in dt is valid, so we don't have * to sanity check this elsewhere: */ if (WARN_ON(asid > qcom_iommu->max_asid) || - WARN_ON(qcom_iommu->ctxs[asid] == NULL)) { - put_device(&iommu_pdev->dev); + WARN_ON(qcom_iommu->ctxs[asid] == NULL)) return -EINVAL; - }
if (!dev_iommu_priv_get(dev)) { dev_iommu_priv_set(dev, qcom_iommu); @@ -574,10 +574,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) * multiple different iommu devices. Multiple context * banks are ok, but multiple devices are not: */ - if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) { - put_device(&iommu_pdev->dev); + if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) return -EINVAL; - } }
return iommu_fwspec_add_ids(dev, &asid, 1);
linux-stable-mirror@lists.linaro.org
-
gregkh＠linuxfoundation.org -
Sasha Levin