[PATCH v2 0/3] Misc. SCM driver fixes

[PATCH v2 0/3] Misc. SCM driver fixes

[Mukesh Ojha]

Some of the existing issue reported by Sasiko mentioned here

https://lore.kernel.org/all/20260624192213.C82691F000E9@smtp.kernel.org/

https://lore.kernel.org/all/20260624192418.92B761F000E9@smtp.kernel.org/

and the series is about addressing them.


Changes in v2: https://lore.kernel.org/lkml/20260625093644.3918184-1-mukesh.ojha@oss.qualcomm.com/
 - Separated the fixes(2/3, 3/3) as per review.
 - Added 1/3 as new patch.

Mukesh Ojha (3):
  firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm
    is published
  firmware: qcom: scm: Fix reserved memory cleanup on probe failure
  firmware: qcom: scm: Fix tzmem state on probe retry

 drivers/firmware/qcom/qcom_scm-smc.c |  2 +-
 drivers/firmware/qcom/qcom_scm.c     | 43 ++++++++++++++++------------
 drivers/firmware/qcom/qcom_scm.h     |  2 +-
 drivers/firmware/qcom/qcom_tzmem.c   | 13 ++++++---
 4 files changed, 36 insertions(+), 24 deletions(-)

-- 
2.53.0

[PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published

[Mukesh Ojha]

In qcom_scm_probe(), devm_request_threaded_irq() is called before
smp_store_release(&__scm, scm). Two paths can dereference __scm before
it is published, both causing a NULL pointer dereference.

The IRQ handler receives scm via its data argument but passes only wq_ctx
to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then
dereference __scm directly. Thread scm through both functions so the IRQ
handler path never touches __scm.

Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via
qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing
qcom_scm_wait_for_wq_completion() to run before __scm is published and
dereference it. Add platform_set_drvdata(pdev, scm) early in probe and
change qcom_scm_wait_for_wq_completion() to take the device pointer and
use dev_get_drvdata() to reach scm, removing any dependency on __scm.

Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm-smc.c |  2 +-
 drivers/firmware/qcom/qcom_scm.c     | 22 ++++++++++------------
 drivers/firmware/qcom/qcom_scm.h     |  2 +-
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
index 574930729ddd..7abe60fce676 100644
--- a/drivers/firmware/qcom/qcom_scm-smc.c
+++ b/drivers/firmware/qcom/qcom_scm-smc.c
@@ -105,7 +105,7 @@ static int __scm_smc_do_quirk_handle_waitq(struct device *dev, struct arm_smccc_
 			wq_ctx = res->a1;
 			smc_call_ctx = res->a2;
 
-			ret = qcom_scm_wait_for_wq_completion(wq_ctx);
+			ret = qcom_scm_wait_for_wq_completion(dev, wq_ctx);
 			if (ret)
 				return ret;
 
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 6b601a4b89db..464ae3b4ca43 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2630,23 +2630,20 @@ static int qcom_scm_get_waitq_irq(struct qcom_scm *scm)
 	return irq_create_fwspec_mapping(&fwspec);
 }
 
-static struct completion *qcom_scm_get_completion(u32 wq_ctx)
+static struct completion *qcom_scm_get_completion(struct qcom_scm *scm, u32 wq_ctx)
 {
-	struct completion *wq;
-
-	if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt))
+	if (WARN_ON_ONCE(wq_ctx >= scm->wq_cnt))
 		return ERR_PTR(-EINVAL);
 
-	wq = &__scm->waitq_comps[wq_ctx];
-
-	return wq;
+	return &scm->waitq_comps[wq_ctx];
 }
 
-int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
+int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx)
 {
+	struct qcom_scm *scm = dev_get_drvdata(dev);
 	struct completion *wq;
 
-	wq = qcom_scm_get_completion(wq_ctx);
+	wq = qcom_scm_get_completion(scm, wq_ctx);
 	if (IS_ERR(wq))
 		return PTR_ERR(wq);
 
@@ -2655,11 +2652,11 @@ int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
 	return 0;
 }
 
-static int qcom_scm_waitq_wakeup(unsigned int wq_ctx)
+static int qcom_scm_waitq_wakeup(struct qcom_scm *scm, unsigned int wq_ctx)
 {
 	struct completion *wq;
 
-	wq = qcom_scm_get_completion(wq_ctx);
+	wq = qcom_scm_get_completion(scm, wq_ctx);
 	if (IS_ERR(wq))
 		return PTR_ERR(wq);
 
@@ -2686,7 +2683,7 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
 			goto out;
 		}
 
-		ret = qcom_scm_waitq_wakeup(wq_ctx);
+		ret = qcom_scm_waitq_wakeup(scm, wq_ctx);
 		if (ret)
 			goto out;
 	} while (more_pending);
@@ -2746,6 +2743,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	scm->dev = &pdev->dev;
+	platform_set_drvdata(pdev, scm);
 	ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index caab80a73e17..cf90a565fdfb 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -66,7 +66,7 @@ struct qcom_scm_res {
 	u64 result[MAX_QCOM_SCM_RETS];
 };
 
-int qcom_scm_wait_for_wq_completion(u32 wq_ctx);
+int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx);
 int scm_get_wq_ctx(u32 *wq_ctx, u32 *flags, u32 *more_pending);
 
 #define SCM_SMC_FNID(s, c)	((((s) & 0xFF) << 8) | ((c) & 0xFF))
-- 
2.53.0

[PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure

[Mukesh Ojha]

of_reserved_mem_device_init() adds an entry to a global list with no
devres counterpart. If qcom_scm_probe() fails after the call the
assignment is never cleaned up. A probe retry would add a duplicate
entry, leaking the original one permanently.

Add an err_rmem label that calls of_reserved_mem_device_release() and
route all error paths after of_reserved_mem_device_init() through it.
of_reserved_mem_device_release() is safe to call unconditionally as it
simply walks an empty list when nothing was assigned.

Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 464ae3b4ca43..f0e19fc314b4 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2785,9 +2785,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) {
+		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;
@@ -2795,9 +2797,12 @@ 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 = PTR_ERR(scm->mempool);
+		dev_err_probe(scm->dev, ret,
+			      "Failed to create the SCM memory pool\n");
+		goto err_rmem;
+	}
 
 	ret = qcom_scm_query_waitq_count(scm);
 	scm->wq_cnt = ret < 0 ? QCOM_SCM_DEFAULT_WAITQ_COUNT : ret;
@@ -2868,6 +2873,10 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	qcom_scm_gunyah_wdt_init(scm);
 
 	return 0;
+
+err_rmem:
+	of_reserved_mem_device_release(scm->dev);
+	return ret;
 }
 
 static void qcom_scm_shutdown(struct platform_device *pdev)
-- 
2.53.0

[PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry

[Mukesh Ojha]

qcom_tzmem_enable() returns -EBUSY if called a second time, but this
causes probe retries to fail permanently if a later step in
qcom_scm_probe() defers after qcom_tzmem_enable() has already succeeded.

Use DO_ONCE() to ensure qcom_tzmem_init() runs exactly once across all
calls in a thread-safe manner. qcom_tzmem_dev is set on every call since
probe retries use the same device pointer. The result of the first
initialisation is cached and returned to every subsequent caller.

Fixes: 40289e35ca52 ("firmware: qcom: scm: enable the TZ mem allocator")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_tzmem.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
index 0635cbeacfc8..0fd9581275f1 100644
--- a/drivers/firmware/qcom/qcom_tzmem.c
+++ b/drivers/firmware/qcom/qcom_tzmem.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/mm.h>
+#include <linux/once.h>
 #include <linux/radix-tree.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -507,14 +508,18 @@ phys_addr_t qcom_tzmem_to_phys(void *vaddr)
 }
 EXPORT_SYMBOL_GPL(qcom_tzmem_to_phys);
 
+static void qcom_tzmem_do_init(int *result)
+{
+	*result = qcom_tzmem_init();
+}
+
 int qcom_tzmem_enable(struct device *dev)
 {
-	if (qcom_tzmem_dev)
-		return -EBUSY;
+	static int result;
 
 	qcom_tzmem_dev = dev;
-
-	return qcom_tzmem_init();
+	DO_ONCE(qcom_tzmem_do_init, &result);
+	return result;
 }
 EXPORT_SYMBOL_GPL(qcom_tzmem_enable);
 
-- 
2.53.0

Re: [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published

[Konrad Dybcio]

On 6/29/26 4:17 PM, Mukesh Ojha wrote:
> In qcom_scm_probe(), devm_request_threaded_irq() is called before
> smp_store_release(&__scm, scm). Two paths can dereference __scm before
> it is published, both causing a NULL pointer dereference.
> 
> The IRQ handler receives scm via its data argument but passes only wq_ctx
> to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then
> dereference __scm directly. Thread scm through both functions so the IRQ
> handler path never touches __scm.
> 
> Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via
> qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing
> qcom_scm_wait_for_wq_completion() to run before __scm is published and
> dereference it. Add platform_set_drvdata(pdev, scm) early in probe and
> change qcom_scm_wait_for_wq_completion() to take the device pointer and
> use dev_get_drvdata() to reach scm, removing any dependency on __scm.
> 
> Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

[...]

> -int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
> +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx)

I think it logically makes more sense to pass a struct qcom_scm* here
and get the data from the caller, but potayto/potato

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published

[Mukesh Ojha]

On Tue, Jun 30, 2026 at 11:50:30AM +0200, Konrad Dybcio wrote:
> On 6/29/26 4:17 PM, Mukesh Ojha wrote:
> > In qcom_scm_probe(), devm_request_threaded_irq() is called before
> > smp_store_release(&__scm, scm). Two paths can dereference __scm before
> > it is published, both causing a NULL pointer dereference.
> > 
> > The IRQ handler receives scm via its data argument but passes only wq_ctx
> > to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then
> > dereference __scm directly. Thread scm through both functions so the IRQ
> > handler path never touches __scm.
> > 
> > Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via
> > qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing
> > qcom_scm_wait_for_wq_completion() to run before __scm is published and
> > dereference it. Add platform_set_drvdata(pdev, scm) early in probe and
> > change qcom_scm_wait_for_wq_completion() to take the device pointer and
> > use dev_get_drvdata() to reach scm, removing any dependency on __scm.
> > 
> > Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> > -int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
> > +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx)
> 
> I think it logically makes more sense to pass a struct qcom_scm* here
> and get the data from the caller, but potayto/potato

To maintain the abstraction of struct qcom_scm* to
qcom_scm-smc.c, had to use a different way.

-- 
-Mukesh Ojha

Re: [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure

[Konrad Dybcio]

On 6/29/26 4:17 PM, Mukesh Ojha wrote:
> of_reserved_mem_device_init() adds an entry to a global list with no
> devres counterpart. If qcom_scm_probe() fails after the call the
> assignment is never cleaned up. A probe retry would add a duplicate
> entry, leaking the original one permanently.
> 
> Add an err_rmem label that calls of_reserved_mem_device_release() and
> route all error paths after of_reserved_mem_device_init() through it.
> of_reserved_mem_device_release() is safe to call unconditionally as it
> simply walks an empty list when nothing was assigned.
> 
> Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout")
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 464ae3b4ca43..f0e19fc314b4 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -2785,9 +2785,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) {
> +		dev_err_probe(scm->dev, ret,
> +			      "Failed to enable the TrustZone memory allocator\n");

Assign ret = dev_err_probe to preserve the usefulness of this API

Alternatively, I cobbled this together, feel free to take it forward
(compile-tested only):

From 917c849fd21c6660ba0bd55f8b6ce4cb5dfc8299 Mon Sep 17 00:00:00 2001
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Date: Tue, 30 Jun 2026 12:02:16 +0200
Subject: [PATCH] of: reserved_mem: Introduce devres-managed initialization
 functions

Introduce devres-based helpers for of_reserved_mem_device_init(_by_idx)
to help fight dangling references and ever so slightly reduce the
number of boilerplate deinitialization calls.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/of/of_reserved_mem.c    | 37 +++++++++++++++++++++++++++++++++
 include/linux/of_reserved_mem.h | 25 ++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 82222bd45ac6..79fa04d4cf04 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -787,6 +787,43 @@ void of_reserved_mem_device_release(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(of_reserved_mem_device_release);
 
+static void devm_of_reserved_mem_device_release(struct device *dev, void *res)
+{
+	of_reserved_mem_device_release(*(struct device **)res);
+}
+
+/**
+ * devm_of_reserved_mem_device_init_by_idx - Resource managed of_reserved_mem_device_init_by_idx()
+ * @dev: Pointer to the device to configure
+ *
+ * This function assigns respective DMA-mapping operations based on the first
+ * reserved memory region specified by 'memory-region' property in device tree
+ * node of the given device.
+ *
+ * Returns: Negative errno on failure or zero on success.
+ */
+int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx)
+{
+	struct device **ptr;
+	int ret;
+
+	ptr = devres_alloc(devm_of_reserved_mem_device_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	ret = of_reserved_mem_device_init_by_idx(dev, np, idx);
+	if (ret) {
+		devres_free(ptr);
+		return ret;
+	}
+
+	*ptr = dev;
+	devres_add(dev, ptr);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_of_reserved_mem_device_init_by_idx);
+
 /**
  * of_reserved_mem_lookup() - acquire reserved_mem from a device node
  * @np:		node pointer of the desired reserved-memory region
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index e8b20b29fa68..28beeeb91f4e 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -47,6 +47,8 @@ int of_reserved_mem_region_to_resource_byname(const struct device_node *np,
 					      const char *name, struct resource *res);
 int of_reserved_mem_region_count(const struct device_node *np);
 
+int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx);
+
 #else
 
 #define RESERVEDMEM_OF_DECLARE(name, compat, ops)			\
@@ -91,6 +93,14 @@ static inline int of_reserved_mem_region_count(const struct device_node *np)
 {
 	return 0;
 }
+
+static inline int devm_of_reserved_mem_device_init_by_idx(struct device *dev,
+							  struct device_node *np,
+							  int idx)
+{
+	return -ENOSYS;
+}
+
 #endif
 
 /**
@@ -108,4 +118,19 @@ static inline int of_reserved_mem_device_init(struct device *dev)
 	return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
 }
 
+/**
+ * of_reserved_mem_device_init() - Devres-managed version of of_reserved_mem_device_init()
+ * @dev:	Pointer to the device to configure
+ *
+ * This function assigns respective DMA-mapping operations based on the first
+ * reserved memory region specified by 'memory-region' property in device tree
+ * node of the given device.
+ *
+ * Returns error code or zero on success.
+ */
+static inline int devm_of_reserved_mem_device_init(struct device *dev)
+{
+	return devm_of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
+}
+
 #endif /* __OF_RESERVED_MEM_H */
-- 
2.54.0


Konrad

Re: [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry

[Konrad Dybcio]

On 6/29/26 4:17 PM, Mukesh Ojha wrote:
> qcom_tzmem_enable() returns -EBUSY if called a second time, but this
> causes probe retries to fail permanently if a later step in
> qcom_scm_probe() defers after qcom_tzmem_enable() has already succeeded.
> 
> Use DO_ONCE() to ensure qcom_tzmem_init() runs exactly once across all
> calls in a thread-safe manner. qcom_tzmem_dev is set on every call since
> probe retries use the same device pointer. The result of the first
> initialisation is cached and returned to every subsequent caller.
> 
> Fixes: 40289e35ca52 ("firmware: qcom: scm: enable the TZ mem allocator")
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2 0/3] Misc. SCM driver fixes

[Bartosz Golaszewski]

On Mon, 29 Jun 2026 16:17:47 +0200, Mukesh Ojha
<mukesh.ojha@oss.qualcomm.com> said:
> Some of the existing issue reported by Sasiko mentioned here
>
> https://lore.kernel.org/all/20260624192213.C82691F000E9@smtp.kernel.org/
>
> https://lore.kernel.org/all/20260624192418.92B761F000E9@smtp.kernel.org/
>
> and the series is about addressing them.
>
>
> Changes in v2: https://lore.kernel.org/lkml/20260625093644.3918184-1-mukesh.ojha@oss.qualcomm.com/
>  - Separated the fixes(2/3, 3/3) as per review.
>  - Added 1/3 as new patch.
>
> Mukesh Ojha (3):
>   firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm
>     is published
>   firmware: qcom: scm: Fix reserved memory cleanup on probe failure
>   firmware: qcom: scm: Fix tzmem state on probe retry
>
>  drivers/firmware/qcom/qcom_scm-smc.c |  2 +-
>  drivers/firmware/qcom/qcom_scm.c     | 43 ++++++++++++++++------------
>  drivers/firmware/qcom/qcom_scm.h     |  2 +-
>  drivers/firmware/qcom/qcom_tzmem.c   | 13 ++++++---
>  4 files changed, 36 insertions(+), 24 deletions(-)
>
> --
> 2.53.0
>
>

With the issue raised by Konrad for patch 2/3 fixed:

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure

[Mukesh Ojha]

On Tue, Jun 30, 2026 at 12:07:26PM +0200, Konrad Dybcio wrote:
> On 6/29/26 4:17 PM, Mukesh Ojha wrote:
> > of_reserved_mem_device_init() adds an entry to a global list with no
> > devres counterpart. If qcom_scm_probe() fails after the call the
> > assignment is never cleaned up. A probe retry would add a duplicate
> > entry, leaking the original one permanently.
> > 
> > Add an err_rmem label that calls of_reserved_mem_device_release() and
> > route all error paths after of_reserved_mem_device_init() through it.
> > of_reserved_mem_device_release() is safe to call unconditionally as it
> > simply walks an empty list when nothing was assigned.
> > 
> > Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> >  drivers/firmware/qcom/qcom_scm.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index 464ae3b4ca43..f0e19fc314b4 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -2785,9 +2785,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) {
> > +		dev_err_probe(scm->dev, ret,
> > +			      "Failed to enable the TrustZone memory allocator\n");
> 
> Assign ret = dev_err_probe to preserve the usefulness of this API
>

Sure.

> Alternatively, I cobbled this together, feel free to take it forward
> (compile-tested only):


Sure, thank you along with all the clean up.

-Mukesh
> 
> From 917c849fd21c6660ba0bd55f8b6ce4cb5dfc8299 Mon Sep 17 00:00:00 2001
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Date: Tue, 30 Jun 2026 12:02:16 +0200
> Subject: [PATCH] of: reserved_mem: Introduce devres-managed initialization
>  functions
> 
> Introduce devres-based helpers for of_reserved_mem_device_init(_by_idx)
> to help fight dangling references and ever so slightly reduce the
> number of boilerplate deinitialization calls.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  drivers/of/of_reserved_mem.c    | 37 +++++++++++++++++++++++++++++++++
>  include/linux/of_reserved_mem.h | 25 ++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 82222bd45ac6..79fa04d4cf04 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -787,6 +787,43 @@ void of_reserved_mem_device_release(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(of_reserved_mem_device_release);
>  
> +static void devm_of_reserved_mem_device_release(struct device *dev, void *res)
> +{
> +	of_reserved_mem_device_release(*(struct device **)res);
> +}
> +
> +/**
> + * devm_of_reserved_mem_device_init_by_idx - Resource managed of_reserved_mem_device_init_by_idx()
> + * @dev: Pointer to the device to configure
> + *
> + * This function assigns respective DMA-mapping operations based on the first
> + * reserved memory region specified by 'memory-region' property in device tree
> + * node of the given device.
> + *
> + * Returns: Negative errno on failure or zero on success.
> + */
> +int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx)
> +{
> +	struct device **ptr;
> +	int ret;
> +
> +	ptr = devres_alloc(devm_of_reserved_mem_device_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ret = of_reserved_mem_device_init_by_idx(dev, np, idx);
> +	if (ret) {
> +		devres_free(ptr);
> +		return ret;
> +	}
> +
> +	*ptr = dev;
> +	devres_add(dev, ptr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_of_reserved_mem_device_init_by_idx);
> +
>  /**
>   * of_reserved_mem_lookup() - acquire reserved_mem from a device node
>   * @np:		node pointer of the desired reserved-memory region
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index e8b20b29fa68..28beeeb91f4e 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -47,6 +47,8 @@ int of_reserved_mem_region_to_resource_byname(const struct device_node *np,
>  					      const char *name, struct resource *res);
>  int of_reserved_mem_region_count(const struct device_node *np);
>  
> +int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx);
> +
>  #else
>  
>  #define RESERVEDMEM_OF_DECLARE(name, compat, ops)			\
> @@ -91,6 +93,14 @@ static inline int of_reserved_mem_region_count(const struct device_node *np)
>  {
>  	return 0;
>  }
> +
> +static inline int devm_of_reserved_mem_device_init_by_idx(struct device *dev,
> +							  struct device_node *np,
> +							  int idx)
> +{
> +	return -ENOSYS;
> +}
> +
>  #endif
>  
>  /**
> @@ -108,4 +118,19 @@ static inline int of_reserved_mem_device_init(struct device *dev)
>  	return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
>  }
>  
> +/**
> + * of_reserved_mem_device_init() - Devres-managed version of of_reserved_mem_device_init()
> + * @dev:	Pointer to the device to configure
> + *
> + * This function assigns respective DMA-mapping operations based on the first
> + * reserved memory region specified by 'memory-region' property in device tree
> + * node of the given device.
> + *
> + * Returns error code or zero on success.
> + */
> +static inline int devm_of_reserved_mem_device_init(struct device *dev)
> +{
> +	return devm_of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
> +}
> +
>  #endif /* __OF_RESERVED_MEM_H */
> -- 
> 2.54.0
> 
> 
> Konrad

-- 
-Mukesh Ojha