[PATCH AUTOSEL 6.19-6.18] soc: imx8m: Fix error handling for clk_prepare_enable()

[PATCH AUTOSEL 6.19-6.18] soc: imx8m: Fix error handling for clk_prepare_enable()

[Sasha Levin]

From: Peng Fan <peng.fan@nxp.com>

[ Upstream commit f6ef3d9ff81240e9bcc030f2da132eb0f8a761d7 ]

imx8m_soc_prepare() directly returns the result of clk_prepare_enable(),
which skips proper cleanup if the clock enable fails. Check the return
value of clk_prepare_enable() and release resources if failure.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202601111406.ZVV3YaiU-lkp@intel.com/
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Interesting - in v6.15 and earlier, `clk_prepare_enable()` was called
without checking its return value at all, but the code continued with
proper cleanup on the success path. The v6.15 code has a different
(arguably less severe) issue - it doesn't check `clk_prepare_enable()`
return value but doesn't leak resources on normal exit.

Now I have a complete picture. Let me write the analysis.

---

## Detailed Analysis

### 1. Commit Message Analysis

The commit title explicitly says "Fix error handling for
clk_prepare_enable()". The message clearly describes the bug:
`imx8m_soc_prepare()` directly returns the result of
`clk_prepare_enable()`, bypassing the `err_clk` cleanup label when the
clock enable fails. This is a resource leak bug.

**Reporters:** Kernel test robot (lkp) and Dan Carpenter (well-known
Smatch/static analysis developer). This means the bug was found via
static analysis (Smatch), not a runtime crash report. The presence of
"Closes:" link to a kernel test robot report confirms this.

**Reviews:** Two Reviewed-by tags (Marco Felsch and Daniel Baluta),
indicating the fix was validated by subsystem developers.

### 2. Code Change Analysis

The bug is in `imx8m_soc_prepare()`:

```131:156:drivers/soc/imx/soc-imx8m.c
static int imx8m_soc_prepare(struct platform_device *pdev, const char
*ocotp_compatible)
{
        // ... setup code ...
        drvdata->ocotp_base = of_iomap(np, 0);    // resource 1: iomap
        // ...
        drvdata->clk = of_clk_get_by_name(np, NULL);  // resource 2:
clock ref
        // ...
        return clk_prepare_enable(drvdata->clk);   // BUG: if this
fails, ocotp_base is leaked!

err_clk:
        iounmap(drvdata->ocotp_base);
        return ret;
}
```

**The bug mechanism:** When `clk_prepare_enable()` fails, the function
returns directly with the error code, skipping the `err_clk` label which
calls `iounmap(drvdata->ocotp_base)`. This leaks:
1. The `ocotp_base` iomap mapping (definite leak)
2. The clock reference from `of_clk_get_by_name()` (also leaked, since
   `clk_put()` is not called - this is arguably still not fully fixed,
   but the `err_clk` label was designed for the `of_clk_get_by_name`
   failure case where `clk_put` should not be called)

**The fix:** Check the return value, jump to `err_clk` on failure, which
properly unmaps `ocotp_base`:

```diff
- return clk_prepare_enable(drvdata->clk);
+       ret = clk_prepare_enable(drvdata->clk);
+       if (ret)
+               goto err_clk;
+       return 0;
```

**Note:** There is a subtlety - when `clk_prepare_enable()` fails, the
`err_clk` label only calls `iounmap()` but not `clk_put()`. The clock
reference obtained from `of_clk_get_by_name()` is still leaked. However,
this is a separate (pre-existing) concern. The fix as written is still
an improvement: it's strictly better than the old code.

### 3. Dependency/Prerequisite Analysis

This is **critical** for the backport decision:

- The function `imx8m_soc_prepare()` was introduced in commit
  `390c01073f5d0` ("soc: imx8m: Cleanup with adding
  imx8m_soc_[un]prepare"), which first appeared in **v6.16-rc1**.
- This means the **buggy code does NOT exist** in v6.15 or any earlier
  stable tree (v6.14, v6.13, v6.12, v6.6, v6.1, 5.15, etc.).
- The bug was introduced in v6.16 and the fix only applies to
  **v6.16.y** stable and newer (v6.17.y, v6.18.y if applicable).
- I confirmed the buggy code exists identically in v6.16.12 (latest
  v6.16 stable), so the fix would apply cleanly.

### 4. Scope and Risk Assessment

- **Size:** 5 lines changed (+5/-1) in a single file - extremely small
  and surgical.
- **Risk:** Essentially zero. The fix only changes the error path
  behavior (adds proper cleanup). The success path (when
  `clk_prepare_enable()` works) is unchanged (returns 0 instead of the
  return value of `clk_prepare_enable()`, but those are equivalent on
  success).
- **Subsystem:** SoC driver for NXP i.MX8M family, a widely-used
  embedded platform.

### 5. User Impact

- **Affected hardware:** All i.MX8M family SoCs (i.MX8MQ, i.MX8MM,
  i.MX8MN, i.MX8MP) - very common in embedded/industrial/automotive use.
- **Trigger likelihood:** Low in practice. `clk_prepare_enable()` for
  the OCOTP clock is unlikely to fail on real hardware under normal
  conditions.
- **Severity if triggered:** Resource leak (iomap + clock reference).
  Not a crash, not security-critical, not data corruption. On repeated
  failures (e.g., in a test environment or if the clock framework has
  issues), could contribute to resource exhaustion.
- **Detection method:** Static analysis (Smatch/kernel test robot), not
  a user report.

### 6. Classification

This is a **real bug fix** - it fixes a resource leak on an error path.
It falls into the category of "error path changes that fix resource
leaks," which is explicitly identified as backport-worthy material
despite being described as a "cleanup."

### 7. Stable Criteria Check

| Criterion | Assessment |
|---|---|
| Obviously correct and tested | Yes - trivially correct, reviewed by 2
developers |
| Fixes a real bug | Yes - resource leak on error path |
| Important issue | Moderate - resource leak, not crash/corruption |
| Small and contained | Yes - 5 lines in 1 file |
| No new features/APIs | Correct |
| Applies cleanly | Yes - to v6.16.y |

### Conclusion

This is a small, obviously correct fix for a resource leak in the i.MX8M
SoC driver's error handling path. The fix is surgical (5 lines), well-
reviewed, and applies cleanly to v6.16.y stable (the only stable tree
that has the affected code). While the bug is unlikely to trigger in
practice (it requires `clk_prepare_enable()` to fail), the fix has
essentially zero regression risk and properly handles an error path that
should have been handled from the start. The fix was reported by kernel
test robot and Dan Carpenter, two well-known automated and manual static
analysis sources, and was reviewed by two subsystem developers.

The fix is appropriate for v6.16.y stable backport. It does NOT apply to
older stable trees (v6.15.y and below) because the affected function was
introduced in v6.16.

**YES**

 drivers/soc/imx/soc-imx8m.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index 04a1b60f2f2b5..8e2322999f099 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -148,7 +148,11 @@ static int imx8m_soc_prepare(struct platform_device *pdev, const char *ocotp_com
 		goto err_clk;
 	}
 
-	return clk_prepare_enable(drvdata->clk);
+	ret = clk_prepare_enable(drvdata->clk);
+	if (ret)
+		goto err_clk;
+
+	return 0;
 
 err_clk:
 	iounmap(drvdata->ocotp_base);
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.12] firmware: arm_ffa: Unmap Rx/Tx buffers on init failure

[Sasha Levin]

From: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>

[ Upstream commit 9fda364cb78c8b9e1abe4029f877300c94655742 ]

ffa_init() maps the Rx/Tx buffers via ffa_rxtx_map() but on the
partition setup failure path it never unmaps them.

Add the missing ffa_rxtx_unmap() call in the error path so that
the Rx/Tx buffers are properly released before freeing the backing
pages.

Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
Message-Id: <20251210031656.56194-1-lihaoxiang@isrc.iscas.ac.cn>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis

### 1. Commit Message Analysis

The commit message clearly describes a **resource leak on an error
path**: `ffa_init()` maps Rx/Tx buffers via `ffa_rxtx_map()`, but when
`ffa_setup_partitions()` fails, the error path frees the backing pages
without first unmapping the buffers from the firmware. The fix adds the
missing `ffa_rxtx_unmap()` call. The commit was accepted by the FF-A
subsystem maintainer Sudeep Holla.

### 2. Code Change Analysis

The fix is a **single line addition**:

```2071:2071:drivers/firmware/arm_ffa/driver.c
        ffa_rxtx_unmap(drv_info->vm_id);
```

inserted between `ffa_notifications_cleanup()` and the `free_pages:`
label.

**Understanding the bug mechanism:**

The `ffa_init()` function follows this flow:
1. Allocates Rx/Tx buffer pages (lines 2038-2048)
2. Maps them to firmware via `ffa_rxtx_map()` (line 2050) — this is an
   FF-A SMC/HVC call to the secure firmware (e.g., TrustZone / Hafnium
   hypervisor)
3. Sets up partitions via `ffa_setup_partitions()` (line 2065)
4. If partition setup fails, falls through to `free_pages:` which frees
   the backing pages

The problem: Step 4 **never calls `ffa_rxtx_unmap()`**, so the firmware
retains stale references to the freed physical pages. The `ffa_exit()`
function (line 2081-2089) correctly calls `ffa_rxtx_unmap()` before
freeing pages, confirming the error path was simply missing this
cleanup.

**Consequences of the bug:**
- The firmware (running in secure world or EL2 hypervisor) still holds
  mappings to physical addresses of freed kernel pages
- Those freed pages can be reallocated for other kernel purposes
- The firmware could write to these stale mappings, causing **memory
  corruption** of unrelated kernel data
- On systems with a stage-2 hypervisor, the mapping entries are also
  leaked

### 3. Bug Origin

The bug was introduced by commit
`0c565d16b80074e57e3e56240d13fc6cd6ed0334` ("firmware: arm_ffa: Handle
partitions setup failures"), which first made `ffa_setup_partitions()`
return an error and added an error path in `ffa_init()`. Before this
commit, `ffa_setup_partitions()` was void and never triggered error
cleanup. This commit landed in **v6.8**. The error path was later
restructured by `be61da9385766` (v6.15), but the missing unmap persisted
through both versions.

### 4. Classification

This is a **resource leak / firmware-side stale mapping fix** on an
error path. It is NOT a new feature, API change, or refactoring — it is
purely adding missing cleanup.

### 5. Scope and Risk Assessment

- **Lines changed**: 1 line added
- **Files changed**: 1 file (`drivers/firmware/arm_ffa/driver.c`)
- **Risk**: Extremely low. The `ffa_rxtx_unmap()` function is already
  used in `ffa_exit()` for normal cleanup. Adding it to the error path
  is the obvious correct fix. The function itself just issues an FF-A
  call to the firmware.
- **Regression potential**: Near zero. This only affects the failure
  path of `ffa_setup_partitions()`, and calling unmap before freeing is
  strictly correct.

### 6. User Impact

This affects ARM platforms using FF-A (Firmware Framework for Arm) —
notably ARM TrustZone/OP-TEE systems, Arm CCA systems, and virtualized
ARM environments with Hafnium. While `ffa_setup_partitions()` failure is
not the most common path, when it does occur, the consequences (firmware
holding stale page references, potential memory corruption) are severe.

### 7. Stable Kernel Criteria Check

| Criterion | Met? |
|---|---|
| Obviously correct and tested | Yes — mirrors `ffa_exit()` cleanup
exactly |
| Fixes a real bug | Yes — resource leak + potential memory corruption |
| Important issue | Yes — firmware-side stale mapping to freed kernel
pages |
| Small and contained | Yes — 1 line in 1 file |
| No new features | Yes — purely error-path cleanup |
| Applies cleanly | Yes — for v6.8+ trees (may need minor adjustment for
v6.8-v6.14 which used `cleanup_notifs:` label) |

### 8. Dependency Check

The fix depends on `0c565d16b80074e57e3e56240d13fc6cd6ed0334` (v6.8) for
the error path to exist, and on `be61da9385766` (v6.15) for the exact
code structure in the diff. For stable trees between v6.8 and v6.14, a
trivially adjusted backport would be needed (placing the unmap call
inside the `cleanup_notifs:` section instead of inline). The fix is
self-contained and has no other dependencies.

### Conclusion

This is a textbook error-path resource leak fix: a single line adding
missing firmware unmap before freeing the backing pages. The bug leaves
the firmware holding stale references to freed kernel memory, which
could lead to memory corruption. The fix is minimal, obviously correct
(mirrors `ffa_exit()`), and accepted by the subsystem maintainer. It
meets all stable kernel criteria.

**YES**

 drivers/firmware/arm_ffa/driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index c72ee47565856..7209a630f6d16 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -2068,6 +2068,7 @@ static int __init ffa_init(void)
 
 	pr_err("failed to setup partitions\n");
 	ffa_notifications_cleanup();
+	ffa_rxtx_unmap(drv_info->vm_id);
 free_pages:
 	if (drv_info->tx_buffer)
 		free_pages_exact(drv_info->tx_buffer, rxtx_bufsz);
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.12] Revert "arm64: zynqmp: Add an OP-TEE node to the device tree"

[Sasha Levin]

From: Tomas Melin <tomas.melin@vaisala.com>

[ Upstream commit c197179990124f991fca220d97fac56779a02c6d ]

This reverts commit 06d22ed6b6635b17551f386b50bb5aaff9b75fbe.

OP-TEE logic in U-Boot automatically injects a reserved-memory
node along with optee firmware node to kernel device tree.
The injection logic is dependent on that there is no manually
defined optee node. Having the node in zynqmp.dtsi effectively
breaks OP-TEE's insertion of the reserved-memory node, causing
memory access violations during runtime.

Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
Link: https://lore.kernel.org/r/20251125-revert-zynqmp-optee-v1-1-d2ce4c0fcaf6@vaisala.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a comprehensive understanding. Let me compile the full
analysis.

---

## Detailed Analysis

### 1. COMMIT MESSAGE ANALYSIS

This is a **revert** commit. It reverts
`06d22ed6b6635b17551f386b50bb5aaff9b75fbe` ("arm64: zynqmp: Add an OP-
TEE node to the device tree"), which was merged for v6.9-rc1 (January
2024).

The revert message explains a concrete, real-world problem:
- U-Boot's OP-TEE logic automatically injects a `reserved-memory` node
  and `optee` firmware node into the kernel device tree at boot time
- This U-Boot injection logic checks for the **absence** of a manually-
  defined optee node — if one already exists, it skips injection
  entirely
- The original commit added a static optee node to `zynqmp.dtsi`, which
  prevents U-Boot from injecting the `reserved-memory` node
- Without the reserved-memory node, **memory access violations occur
  during runtime**

This is not speculative — the commit describes actual runtime memory
access violations.

### 2. CODE CHANGE ANALYSIS

The change is extremely small and surgical — it removes exactly 5 lines
(4 lines of DT node content + 1 blank line):

```195:198:arch/arm64/boot/dts/xilinx/zynqmp.dtsi
                optee: optee  {
                        compatible = "linaro,optee-tz";
                        method = "smc";
                };
```

This removes a statically-defined OP-TEE node from the ZynqMP base
device tree include file (`zynqmp.dtsi`). The `optee` label is not
referenced by any other DTS file in the xilinx directory tree, so there
are no broken cross-references.

### 3. BUG MECHANISM

The bug mechanism is an interaction between the Linux kernel device tree
and U-Boot bootloader:

1. **U-Boot's OP-TEE initialization**: U-Boot contains logic that
   detects if OP-TEE is running and, if so, dynamically modifies the FDT
   (Flattened Device Tree) before passing it to Linux. It adds:
   - A `reserved-memory` node describing memory regions reserved for OP-
     TEE
   - The `optee` firmware node pointing to the TEE

2. **Conditional injection**: U-Boot's injection is conditioned on there
   being **no existing optee node** in the DT. If one is found, U-Boot
   assumes the DT is already properly configured and skips injection.

3. **The problem**: The static optee node in `zynqmp.dtsi` has
   `compatible = "linaro,optee-tz"` and `method = "smc"`, but it does
   NOT include the critical `reserved-memory` region. When U-Boot sees
   this node, it skips its injection. The kernel then boots with:
   - An optee node (so the OP-TEE driver probes and starts communicating
     with the secure world)
   - **No reserved-memory** for OP-TEE (so the kernel may allocate and
     use memory that OP-TEE also uses)

4. **Result**: Memory access violations at runtime. The kernel and OP-
   TEE stomp on each other's memory. This is a data corruption and crash
   bug.

### 4. CLASSIFICATION

This is a **device tree fix** that addresses a **runtime memory
corruption/access violation** bug. It falls squarely in the "DT updates
for existing hardware" exception category. The severity is high:
- Memory access violations = potential crashes, data corruption,
  security boundary violations (TEE compromise)
- Affects all ZynqMP boards running OP-TEE with U-Boot

### 5. ORIGINAL COMMIT ANALYSIS

The original commit (06d22ed6b6635) claimed "having the DT node present
doesn't cause any side effects." This turned out to be **wrong** — it
has a significant side effect with U-Boot's standard OP-TEE injection
flow. The original commit was a feature addition ("add it in case
someone tries to load OP-TEE") that turned out to cause a real bug.

### 6. SCOPE AND RISK ASSESSMENT

- **Lines changed**: 5 lines removed (1 file)
- **Risk**: Extremely low — removing a DT node that shouldn't have been
  there
- **Side effects**: None negative. If someone was using the static optee
  node WITHOUT U-Boot injection, they would need to add it back in their
  board-specific DT overlay. However, the standard OP-TEE workflow on
  ZynqMP uses U-Boot injection, and the original commit was explicitly
  designed as a convenience, not a necessity.
- **Regression potential**: Near zero — the only "regression" is
  returning to the pre-6.9 behavior, which worked correctly

### 7. AFFECTED STABLE TREES

The original commit (06d22ed6b6635) is present in:
- v6.9 through v6.19 (all current stable trees from 6.9 onward)
- Critically, it's in **6.12.y** (current LTS tree)

The fix is relevant to 6.12.y, 6.13.y, and any other maintained stable
tree based on 6.9+.

### 8. DEPENDENCY CHECK

The revert has **zero dependencies**. It simply removes 5 lines from a
DTS file. No other code or DT changes are needed. The `optee` label is
not referenced by any other file. The revert will apply cleanly to any
stable tree that has the original commit.

### 9. USER IMPACT

- **Who is affected**: All ZynqMP users running OP-TEE with standard
  U-Boot boot flow
- **Severity**: HIGH — runtime memory access violations can cause:
  - Kernel crashes/panics
  - OP-TEE secure world corruption
  - Silent data corruption
  - Potential security boundary violation (TEE memory exposed to normal
    world)
- **Platform**: Xilinx ZynqMP (common in embedded, industrial,
  automotive applications)

### 10. TRUST INDICATORS

- Submitted by Tomas Melin (Vaisala), indicating real-world usage and
  testing
- Accepted by Michal Simek (AMD/Xilinx SoC maintainer), indicating
  subsystem maintainer approval
- The commit message clearly explains the mechanism and consequences
- Clean revert of a well-identified problematic commit

### CONCLUSION

This is a textbook stable backport candidate:
1. **Fixes a real bug**: Runtime memory access violations on ZynqMP with
   OP-TEE
2. **Obviously correct**: Clean revert of a commit that introduced the
   problem
3. **Small and contained**: 5-line removal from a single DTS file
4. **No new features**: Removes a broken feature addition
5. **Zero risk**: Returns to the pre-6.9 behavior that worked correctly
6. **No dependencies**: Self-contained change
7. **Real-world impact**: Affects ZynqMP users with OP-TEE
   (embedded/industrial platforms)
8. **Applies to active stable trees**: The buggy commit is in 6.12.y LTS
   and other maintained stables

The fix is small, surgical, and meets all stable kernel criteria.

**YES**

 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 938b014ca9231..b55c6b2e8e0e1 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -192,11 +192,6 @@ psci {
 	};
 
 	firmware {
-		optee: optee  {
-			compatible = "linaro,optee-tz";
-			method = "smc";
-		};
-
 		zynqmp_firmware: zynqmp-firmware {
 			compatible = "xlnx,zynqmp-firmware";
 			#power-domain-cells = <1>;
-- 
2.51.0