Re: [PATCH 3/9] firmware: imx: ele: Add API functions for OCOTP fuse access

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A timeout or interrupt during `ele_msg_send_rcv()` leaves a dangling pointer in the asynchronous mailbox callback state (`priv->waiting_rsp_clbk_hdl`), leading to a NULL pointer dereference and a Use-After-Free heap corruption upon a late firmware response.
--

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

firmware: imx: ele: Add API functions for OCOTP fuse access

This commit adds the ele_read_fuse() and ele_write_fuse() functions to the
ELE S400 API to provide read and write access to OCOTP fuse registers. These
are exposed via imx_se_read_fuse() and imx_se_write_fuse() for use by drivers
such as the OCOTP S400 NVMEM driver.

> diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
> index ec718d322abcd..281d223aa1447 100644
> --- a/drivers/firmware/imx/ele_base_msg.c
> +++ b/drivers/firmware/imx/ele_base_msg.c
[ ... ]
> @@ -303,3 +304,124 @@ int ele_debug_dump(struct se_if_priv *priv)
>  
>  	return ret;
>  }
> +
> +static int ele_read_fuse(struct se_if_priv *priv, uint16_t fuse_id, u32 *value)
> +{
> +	struct se_api_msg *tx_msg __free(kfree) = NULL;
> +	struct se_api_msg *rx_msg __free(kfree) = NULL;
> +	int rx_msg_sz = ELE_READ_FUSE_RSP_MSG_SZ;
> +	int ret = 0;
[ ... ]
> +	tx_msg->data[0] = fuse_id;
> +
> +	ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg,
> +			       ELE_READ_FUSE_REQ_MSG_SZ, rx_msg, rx_msg_sz);
> +	if (ret < 0)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but will a late firmware response cause a
use-after-free here if ele_msg_send_rcv() times out or is interrupted?

If ele_msg_send_rcv() returns an error, it clears the dev_ctx but leaves the
dangling rx_msg pointer in the shared callback handle:

drivers/firmware/imx/ele_common.c:ele_msg_send_rcv() {
    ...
    if (priv->waiting_rsp_clbk_hdl.signal_rcvd) {
        err = -EINTR;
        priv->waiting_rsp_clbk_hdl.signal_rcvd = false;
    }
    priv->waiting_rsp_clbk_hdl.dev_ctx = NULL;
    
    return err;
}

Because rx_msg is managed with __free(kfree), it is immediately freed when
ele_read_fuse() returns on this error path.

If a late response arrives afterwards, se_if_rx_callback() will process it.
It first causes a NULL pointer dereference if dynamic debug is enabled:

drivers/firmware/imx/ele_common.c:se_if_rx_callback() {
    ...
    dev_dbg(dev, "Selecting resp waiter:%s ...",
            se_clbk_hdl->dev_ctx->devname);

Then it unconditionally copies the response into the freed rx_msg buffer:

    ...
    memcpy(se_clbk_hdl->rx_msg, msg, se_clbk_hdl->rx_msg_sz);
}

Does waiting_rsp_clbk_hdl need to be fully cleared (including setting rx_msg
and rx_msg_sz to zero) on error paths in ele_msg_send_rcv() to prevent this
memory corruption?

> +
> +	ret = se_val_rsp_hdr_n_status(priv, rx_msg, ELE_READ_FUSE_REQ,
> +				      rx_msg_sz, true);
> +	if (ret)
> +		return ret;
> +
> +	*value = rx_msg->data[1];
> +
> +	return 0;
> +}

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