Re: [PATCH v28 6/7] arm64: dts: imx8ulp: add secure enclave node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Bypass of the fw_busy circuit breaker when a signal interrupts ele_msg_rcv() at or after the timeout deadline, leading to DMA memory corruption.
- [Medium] The `hsm0` (`secure-enclave`) device tree node is missing the mandatory `memory-region` property, causing device tree schema validation to fail and the driver probe to unconditionally abort.
--

--- Patch [6]: [PATCH v28 6/7] arm64: dts: imx8ulp: add secure enclave node ---
commit 241fbc418ec78fd2d90f05f09b33f7f8fd2d525e
Author: Pankaj Gupta <pankaj.gupta@nxp.com>

arm64: dts: imx8ulp: add secure enclave node
    
Add support for NXP secure enclave called EdgeLock Enclave firmware (se-fw)
for imx8ulp-evk.
    
Add label sram0 for sram@2201f000 and add secure-enclave node

> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index c6d1bb9edf388..182f1444b6434 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
[ ... ]
> @@ -185,6 +185,13 @@ scmi_sensor: protocol@15 {
>  				#thermal-sensor-cells = <1>;
>  			};
>  		};
> +
> +		hsm0: secure-enclave {
> +			compatible = "fsl,imx8ulp-se-ele-hsm";
> +			mbox-names = "tx", "rx";
> +			mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
> +			sram = <&sram0>;
> +		};

[Severity: Medium]
Does this node need a memory-region property to probe successfully?

Looking at se_if_probe() in drivers/firmware/imx/se_ctrl.c, when the
compatible string has reserved_dma_ranges set to true (as this one does),
it calls of_reserved_mem_device_init().

If the memory-region property is missing, that function returns -ENODEV,
which the driver converts into a probe error:

	if (if_node->reserved_dma_ranges) {
		ret = of_reserved_mem_device_init(dev);
		if (ret)
			return dev_err_probe(dev, ret,
					    "Failed to init reserved memory region.");
	}

Will this cause the secure enclave device to unconditionally fail
initialization out-of-the-box?

[Severity: Critical]
Is it possible for the driver's firmware busy circuit breaker to be bypassed
when communicating with this device?

Looking at ele_msg_rcv() in drivers/firmware/imx/ele_common.c, if a signal
interrupts wait_for_completion_interruptible_timeout() at or after the
timeout deadline, an early break bypasses the circuit breaker:

		if (is_rsp_wait_with_timeout) {
			if (time_after_eq(jiffies, deadline_jiffies)) {
				ret = -ETIMEDOUT;
				break;
			}
			remaining_jiffies = deadline_jiffies - jiffies;
		}

Because this breaks early, the loop skips the call to se_mark_fw_busy().

Later, in se_dev_ctx_shared_mem_cleanup() in drivers/firmware/imx/se_ctrl.c,
since the fw_busy flag wasn't set, the DMA memory allocator resets the buffer
position to 0:

	is_fw_busy_dev_ctx = se_is_fw_busy_ctx(dev_ctx);

	/* ... */

	if (!is_fw_busy_dev_ctx)
		se_shared_mem_mgmt->non_secure_mem.pos = 0;

Could this allow the active firmware DMA to overwrite the memory of
subsequent requests since the firmware is still running and will DMA
to the shared buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=6