Re: [PATCH 7/9] nvmem: imx-ocotp-ele: Remove the FUSE_ELE type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] ele_msg_send_rcv() fails to clear waiting_rsp_clbk_hdl.rx_msg on error/timeout paths, leading to heap corruption and NULL pointer dereference by the mailbox callback.
- [High] Missing kfree() on the error path in imx_ocotp_reg_read() causes a memory leak of the dynamically allocated read buffer.
- [High] Missing device link between the OCOTP driver and the Secure Enclave (SE) device creates a Use-After-Free risk for priv->se_data.
--

commit 59602650186437e4e8ff55bf4112fcec530ec2ab
Author: Frieder Schrempf <frieder.schrempf@kontron.de>

nvmem: imx-ocotp-ele: Remove the FUSE_ELE type

There is no use in tracking the fuses that are only accessible via
ELE API and the current lists are incomplete. If the ELE API is
available it will be used as primary access method anyway, otherwise
the fuses not listed as accessible through the FSB can be considered
invalid.

> diff --git a/drivers/nvmem/imx-ocotp-ele.c b/drivers/nvmem/imx-ocotp-ele.c
> index 92cc061b9b347..3d6fad149ed5d 100644
> --- a/drivers/nvmem/imx-ocotp-ele.c
> +++ b/drivers/nvmem/imx-ocotp-ele.c
[ ... ]
> @@ -111,7 +110,7 @@ static int imx_ocotp_reg_read(void *context, unsigned int offset, void *val, siz
>  		}
>  
>  		type = imx_ocotp_fuse_type(context, i);
> -		if (type == FUSE_INVALID || type == FUSE_ELE) {
> +		if (type == FUSE_INVALID) {

[Severity: High]
This isn't a bug introduced by this patch, but looking at the earlier
part of imx_ocotp_reg_read() where the SE device is checked:

	if (priv->se_data) {
		ret = imx_se_read_fuse(priv->se_data, i, buf++);
		if (ret) {
			mutex_unlock(&priv->lock);
			return ret;
		}

Does this error path leak the dynamically allocated read buffer 'p'?

The function returns immediately without calling kfree(p). This could
allow repeated failing reads, such as from nvmem sysfs, to cause memory
exhaustion.

[ ... ]

[Severity: High]
This is a pre-existing issue, but looking at imx_ele_ocotp_probe() in
drivers/nvmem/imx-ocotp-ele.c:

	priv->se_data = platform_get_drvdata(priv->se_dev);
	if (!priv->se_data)
		return dev_err_probe(&pdev->dev, -EPROBE_DEFER,
				     "SE device not ready\n");

Is there a use-after-free risk here due to the missing device link?

The driver caches the SE device's driver data locally as priv->se_data
without establishing a device link (e.g., using device_link_add()).

If the Secure Enclave driver unbinds independently, its private data is
freed. Because this driver remains active, could it pass the dangling
priv->se_data pointer to imx_se_read_fuse() during a subsequent read
or write?

[Severity: Critical]
This is a pre-existing issue, but looking at ele_msg_send_rcv() in
drivers/firmware/imx/ele_common.c:

	err = ele_msg_send(dev_ctx, tx_msg, tx_msg_sz);
	if (err < 0)
		return err;

Could this error path leave a dangling pointer in the global state?

The function returns without clearing priv->waiting_rsp_clbk_hdl.rx_msg
or dev_ctx.

If a spurious response arrives later, se_if_rx_callback() copies data
into the freed rx_msg via memcpy(), which could cause heap corruption:

	dev_dbg(dev, "Selecting resp waiter:%s for mesg header:0x%x.",
		se_clbk_hdl->dev_ctx->devname, *(u32 *)header);
	...
	memcpy(se_clbk_hdl->rx_msg, msg, se_clbk_hdl->rx_msg_sz);

Additionally, if the message times out, dev_ctx is set to NULL but
rx_msg is not.

Would a delayed response crash in se_if_rx_callback() by dereferencing
se_clbk_hdl->dev_ctx->devname without checking if dev_ctx is NULL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-upstreaming-next-20260609-imx-ocotp-ele-v1-0-cb7f3698c3e6@kontron.de?part=7