All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] firmware: qcom: scm: Fix resource cleanup on probe failure
@ 2026-06-25  9:36 Mukesh Ojha
  2026-06-25 10:06 ` Bartosz Golaszewski
  2026-08-15 17:36 ` kernel test robot
  0 siblings, 2 replies; 5+ messages in thread
From: Mukesh Ojha @ 2026-06-25  9:36 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski,
	Elliot Berman, Andrew Halaney
  Cc: linux-arm-msm, linux-kernel, Mukesh Ojha

qcom_scm_probe() acquires two non-devres resources that are never
released if probe fails or defers after them. of_reserved_mem_device_init()
adds an entry to a global list with no devres counterpart, so a retry
would add a duplicate entry and leak the original. qcom_tzmem_enable()
sets a static qcom_tzmem_dev pointer and may set qcom_tzmem_using_shm_bridge;
without cleanup a probe retry finds qcom_tzmem_dev already set and
returns -EBUSY, permanently preventing the driver from probing.

Introduce err_tzmem and err_rmem goto labels at the end of probe to
call qcom_tzmem_disable() and of_reserved_mem_device_release() in the
right order, route all subsequent error paths through them, and add
qcom_tzmem_disable() to qcom_tzmem.c to clear the static state.

Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout")
Fixes: 40289e35ca52 ("firmware: qcom: scm: enable the TZ mem allocator")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
This is reported on sasiko review as existing issue here
 https://lore.kernel.org/all/20260624192213.C82691F000E9@smtp.kernel.org/
and it can go independently.

 drivers/firmware/qcom/qcom_scm.c   | 42 +++++++++++++++++++++---------
 drivers/firmware/qcom/qcom_tzmem.c |  7 +++++
 drivers/firmware/qcom/qcom_tzmem.h |  1 +
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index ba5cdeed8a04..cb3d776fa645 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2883,9 +2883,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
 				     "Failed to setup the reserved memory region for TZ mem\n");
 
 	ret = qcom_tzmem_enable(scm->dev);
-	if (ret)
-		return dev_err_probe(scm->dev, ret,
-				     "Failed to enable the TrustZone memory allocator\n");
+	if (ret) {
+		ret = dev_err_probe(scm->dev, ret,
+				    "Failed to enable the TrustZone memory allocator\n");
+		goto err_rmem;
+	}
 
 	memset(&pool_config, 0, sizeof(pool_config));
 	pool_config.initial_size = 0;
@@ -2893,16 +2895,20 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	pool_config.max_size = SZ_256K;
 
 	scm->mempool = devm_qcom_tzmem_pool_new(scm->dev, &pool_config);
-	if (IS_ERR(scm->mempool))
-		return dev_err_probe(scm->dev, PTR_ERR(scm->mempool),
-				     "Failed to create the SCM memory pool\n");
+	if (IS_ERR(scm->mempool)) {
+		ret = dev_err_probe(scm->dev, PTR_ERR(scm->mempool),
+				    "Failed to create the SCM memory pool\n");
+		goto err_tzmem;
+	}
 
 	ret = qcom_scm_query_waitq_count(scm);
 	scm->wq_cnt = ret < 0 ? QCOM_SCM_DEFAULT_WAITQ_COUNT : ret;
 	scm->waitq_comps = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq_comps),
 					GFP_KERNEL);
-	if (!scm->waitq_comps)
-		return -ENOMEM;
+	if (!scm->waitq_comps) {
+		ret = -ENOMEM;
+		goto err_tzmem;
+	}
 
 	for (i = 0; i < scm->wq_cnt; i++)
 		init_completion(&scm->waitq_comps[i]);
@@ -2912,14 +2918,18 @@ static int qcom_scm_probe(struct platform_device *pdev)
 		irq = platform_get_irq_optional(pdev, 0);
 
 	if (irq < 0) {
-		if (irq != -ENXIO)
-			return irq;
+		if (irq != -ENXIO) {
+			ret = irq;
+			goto err_tzmem;
+		}
 	} else {
 		ret = devm_request_threaded_irq(scm->dev, irq, NULL, qcom_scm_irq_handler,
 						IRQF_ONESHOT, "qcom-scm", scm);
-		if (ret < 0)
-			return dev_err_probe(scm->dev, ret,
-					     "Failed to request qcom-scm irq\n");
+		if (ret < 0) {
+			ret = dev_err_probe(scm->dev, ret,
+					    "Failed to request qcom-scm irq\n");
+			goto err_tzmem;
+		}
 	}
 
 	/*
@@ -2966,6 +2976,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	qcom_scm_gunyah_wdt_init(scm);
 
 	return 0;
+
+err_tzmem:
+	qcom_tzmem_disable(scm->dev);
+err_rmem:
+	of_reserved_mem_device_release(scm->dev);
+	return ret;
 }
 
 static void qcom_scm_shutdown(struct platform_device *pdev)
diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
index 0635cbeacfc8..3f2b782f4a94 100644
--- a/drivers/firmware/qcom/qcom_tzmem.c
+++ b/drivers/firmware/qcom/qcom_tzmem.c
@@ -518,6 +518,13 @@ int qcom_tzmem_enable(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(qcom_tzmem_enable);
 
+void qcom_tzmem_disable(struct device *dev)
+{
+	qcom_tzmem_using_shm_bridge = false;
+	qcom_tzmem_dev = NULL;
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_disable);
+
 MODULE_DESCRIPTION("TrustZone memory allocator for Qualcomm firmware drivers");
 MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@linaro.org>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/qcom/qcom_tzmem.h b/drivers/firmware/qcom/qcom_tzmem.h
index 8fa8a3eb940e..0b0f26d4e22e 100644
--- a/drivers/firmware/qcom/qcom_tzmem.h
+++ b/drivers/firmware/qcom/qcom_tzmem.h
@@ -9,5 +9,6 @@
 struct device;
 
 int qcom_tzmem_enable(struct device *dev);
+void qcom_tzmem_disable(struct device *dev);
 
 #endif /* __QCOM_TZMEM_PRIV_H */
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-15 17:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25  9:36 [PATCH] firmware: qcom: scm: Fix resource cleanup on probe failure Mukesh Ojha
2026-06-25 10:06 ` Bartosz Golaszewski
2026-06-25 11:27   ` Mukesh Ojha
2026-06-25 13:18     ` Bartosz Golaszewski
2026-08-15 17:36 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.