[PATCH 0/7] firmware: imx: ele: misc fixes

[PATCH 0/7] firmware: imx: ele: misc fixes

[Peng Fan (OSS)]

Misc fixes when reading the ele driver, including check condition, size
mismatch and etc. More information could be found in each patch commit
log.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (7):
      firmware: imx: ele: Correct check condition in se_if_rx_callback
      firmware: imx: ele: Correct ele_fw_authenticate API
      firmware: imx: ele: Bypass memcpy when ele_get_info() fails
      firmware: imx: ele: simplify SoC device registration
      firmware: imx: ele: Correct check_hdr_exception_for_sz
      firmware: imx: ele: Use dev_err for error report
      firmware: imx: ele: Fix debug dump size handling

 drivers/firmware/imx/ele_base_msg.c | 15 +++++++++++----
 drivers/firmware/imx/ele_common.c   |  4 ++--
 drivers/firmware/imx/se_ctrl.c      | 15 ++++-----------
 3 files changed, 17 insertions(+), 17 deletions(-)
---
base-commit: c1ecb239fa3456529a32255359fc78b69eb9d847
change-id: 20260525-ele-v1-e1da1ffc882e

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/7] firmware: imx: ele: Correct check condition in se_if_rx_callback

[Peng Fan (OSS)]

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

Reverse logic should be used when checking whether response is correct.
The logic should be when size not match and API is not listed in
exception list, return failure.

Fixes: 106ffe5d78ad8 ("firmware: imx: add driver for NXP EdgeLock Enclave")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/ele_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
index e5117a1f19959ec8dcfb313040121e20be0e92be..8daf32eded43c62daf56540b63e292bf0c6c9845 100644
--- a/drivers/firmware/imx/ele_common.c
+++ b/drivers/firmware/imx/ele_common.c
@@ -219,7 +219,7 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
 			se_clbk_hdl->dev_ctx->devname, *(u32 *)header);
 
 		if (rx_msg_sz != se_clbk_hdl->rx_msg_sz &&
-		    check_hdr_exception_for_sz(priv, header)) {
+		    !check_hdr_exception_for_sz(priv, header)) {
 			dev_err(dev,
 				"%s: Rsp to CMD: hdr(0x%x) with different sz(%d != %d).\n",
 				se_clbk_hdl->dev_ctx->devname, *(u32 *)header,

-- 
2.37.1

Re: [PATCH 1/7] firmware: imx: ele: Correct check condition in se_if_rx_callback

[sashiko-bot]

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

New issues:
- [Critical] The patch alters the condition for size mismatch handling by negating `check_hdr_exception_for_sz()`, inadvertently skipping the essential size truncation for exception commands and causing an out-of-bounds read and information leak.
- [Medium] The commit message indicates that the patch should return a failure when the size does not match for non-exception APIs, but the code only logs the error and proceeds.

Pre-existing issues:
- [Critical] Asynchronous mailbox callbacks can trigger a Null Pointer Dereference or cross-thread data corruption if the completion wait times out, is interrupted, or if unsolicited messages arrive.
- [Critical] The IOCTL handler directly dereferences a user-space pointer during header validation, causing memory safety violations and exposing a TOCTOU vulnerability.
- [High] In NVM command handling, the received message size is unconditionally overwritten after being clamped, leading to a heap buffer overflow.
--

commit dbd468c11e8d6f15f1dbfe05daf128166c6de3a4
Author: Peng Fan <peng.fan@nxp.com>

firmware: imx: ele: Correct check condition in se_if_rx_callback

This commit updates the response validation logic in the EdgeLock Enclave
driver to reverse the condition checking whether a command is in the
exception list for variable length responses.

>     Reverse logic should be used when checking whether response is correct.
>     The logic should be when size not match and API is not listed in
>     exception list, return failure.

[Severity: Medium]
The commit message states this should return a failure, but does the code
actually do that?

Looking at the modified code, it only logs a warning with dev_err() and
clamps the size. Execution then continues to memcpy() and complete(),
returning a positive size value to the caller, which the IOCTL treats as a
success. The failure state is never properly propagated.

> diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> index e5117a1f19959..8daf32eded43c 100644
> --- a/drivers/firmware/imx/ele_common.c
> +++ b/drivers/firmware/imx/ele_common.c
> @@ -219,7 +219,7 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
>  			se_clbk_hdl->dev_ctx->devname, *(u32 *)header);
>  
>  		if (rx_msg_sz != se_clbk_hdl->rx_msg_sz &&
> -		    check_hdr_exception_for_sz(priv, header)) {
> +		    !check_hdr_exception_for_sz(priv, header)) {

[Severity: Critical]
By negating this condition, does this inadvertently skip the size clamping
logic for exception commands?

If check_hdr_exception_for_sz() returns true, this block is skipped, which
means se_clbk_hdl->rx_msg_sz retains its potentially large user-controlled
value instead of being truncated to the actual message size.

The subsequent memcpy() at the end of this function will then read past the
valid 1024-byte hardware mailbox buffer (msg) into the kernel heap, copying
out-of-bounds kernel data into rx_msg. This data is eventually returned to
userspace, resulting in an information disclosure.

[Severity: High]
This isn't a bug introduced by this patch, but does the NVM command handling
unconditionally overwrite the clamped size?

Looking earlier in se_if_rx_callback():

		if (rx_msg_sz > MAX_NVM_MSG_LEN) {
			dev_err(dev,
				"%s: CMD-RCVER NVM: hdr(0x%x) with different sz(%d != %d).\n",
				se_clbk_hdl->dev_ctx->devname, *(u32 *)header,
				rx_msg_sz, se_clbk_hdl->rx_msg_sz);

			se_clbk_hdl->rx_msg_sz = MAX_NVM_MSG_LEN;
		}
		se_clbk_hdl->rx_msg_sz = rx_msg_sz;

Because there is no else clause, se_clbk_hdl->rx_msg_sz is immediately
overwritten with the oversized rx_msg_sz. The subsequent memcpy() could then
write the oversized payload into the pre-allocated NVM buffer, which is
strictly sized at MAX_NVM_MSG_LEN, overflowing the kernel heap.

[Severity: Critical]
This is a pre-existing issue, but can this function experience a NULL pointer
dereference if a delayed response arrives after a timeout?

In ele_msg_send_rcv(), if ele_msg_rcv() times out or is interrupted, the
function returns and clears priv->waiting_rsp_clbk_hdl.dev_ctx to NULL,
while the rx_msg buffer is freed by the __free(kfree) attribute in the
ioctl handler.

When a delayed or unsolicited firmware response eventually arrives,
se_if_rx_callback() accesses se_clbk_hdl->dev_ctx->devname, directly
dereferencing the NULL dev_ctx. Could this cause a kernel panic, or lead to
cross-thread data corruption and a Use-After-Free if another thread is
currently waiting?

[Severity: Critical]
This is a pre-existing issue, but does the IOCTL handler directly
dereference a user-space pointer during header validation?

In se_ioctl_cmd_snd_rcv_rsp_handler(), cmd_snd_rcv_rsp_info.tx_buf is a u64
representing a user-space memory address. It is passed directly to
se_chk_tx_msg_hdr(), which casts it to a struct se_msg_hdr * and dereferences
it without a prior copy_from_user().

Wouldn't this cause an immediate kernel panic on systems with SMAP or PAN
enabled? Additionally, it allows a time-of-check to time-of-use (TOCTOU)
attack where a malicious thread can alter the command ID after the validation
check but before memdup_user(), bypassing validation.

>  			dev_err(dev,
>  				"%s: Rsp to CMD: hdr(0x%x) with different sz(%d != %d).\n",
>  				se_clbk_hdl->dev_ctx->devname, *(u32 *)header,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=1

[PATCH 2/7] firmware: imx: ele: Correct ele_fw_authenticate API

[Peng Fan (OSS)]

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

The command format is as:
0(32bits): Header
1(32bits): Address of the container header
2(32bits): Reserved for future
3(32bits): Actual address of the container header

Correct the data field to only use lower 32bits. If upper 32bits are
not zero, fail the command.

Fixes: 106ffe5d78ad8 ("firmware: imx: add driver for NXP EdgeLock Enclave")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/ele_base_msg.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
index 23f68076b85ce3ac0293bb98e39310e20834f7db..f6346f15450963b9dd8d2df7d3e6b2ce6b4602ce 100644
--- a/drivers/firmware/imx/ele_base_msg.c
+++ b/drivers/firmware/imx/ele_base_msg.c
@@ -208,6 +208,11 @@ int ele_fw_authenticate(struct se_if_priv *priv, phys_addr_t contnr_addr,
 	if (!priv)
 		return -EINVAL;
 
+	if (upper_32_bits(contnr_addr) || upper_32_bits(img_addr)) {
+		dev_err(priv->dev, "Wrong address: %pap %pap\n", &contnr_addr, &img_addr);
+		return -EINVAL;
+	}
+
 	struct se_api_msg *tx_msg __free(kfree)	=
 		kzalloc(ELE_FW_AUTH_REQ_SZ, GFP_KERNEL);
 	if (!tx_msg)
@@ -224,8 +229,8 @@ int ele_fw_authenticate(struct se_if_priv *priv, phys_addr_t contnr_addr,
 		return ret;
 
 	tx_msg->data[0] = lower_32_bits(contnr_addr);
-	tx_msg->data[1] = upper_32_bits(contnr_addr);
-	tx_msg->data[2] = img_addr;
+	tx_msg->data[1] = 0;
+	tx_msg->data[2] = lower_32_bits(img_addr);
 
 	ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_FW_AUTH_REQ_SZ, rx_msg,
 			       ELE_FW_AUTH_RSP_MSG_SZ);

-- 
2.37.1

Re: [PATCH 2/7] firmware: imx: ele: Correct ele_fw_authenticate API

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Dangling pointers left in the global callback handle on error paths lead to Use-After-Free heap corruption or a NULL pointer dereference when a delayed/spurious response arrives.
- [Critical] Uninitialized physical address used for IMEM export/import.
- [High] Silent physical address truncation in `ele_service_swap()`.
- [High] The `ele_msg_rcv()` function causes an infinite busy loop (CPU lockup) if the calling task receives a signal.
--

commit f74ec8f682c5afa2812159200b4566d0c747b041
Author: Peng Fan <peng.fan@nxp.com>
Subject: firmware: imx: ele: Correct ele_fw_authenticate API

This commit corrects the ele_fw_authenticate API by enforcing that the upper
32 bits of both the container and image addresses are zero. It fails the
command if they are not, and correctly populates only the lower 32 bits in
the message data.

> diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
> index 23f68076b85ce..f6346f1545096 100644
> --- a/drivers/firmware/imx/ele_base_msg.c
> +++ b/drivers/firmware/imx/ele_base_msg.c
> @@ -208,6 +208,11 @@ int ele_fw_authenticate(struct se_if_priv *priv, phys_addr_t contnr_addr,
>  	if (!priv)
>  		return -EINVAL;
>  
> +	if (upper_32_bits(contnr_addr) || upper_32_bits(img_addr)) {
> +		dev_err(priv->dev, "Wrong address: %pap %pap\n", &contnr_addr, &img_addr);
> +		return -EINVAL;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but while you are correctly adding bounds
checks here, does ele_service_swap() also need a similar fix?

It appears ele_service_swap() accepts a 64-bit phys_addr_t and silently
truncates it by assigning only lower_32_bits(addr) to the message, ignoring
the upper 32 bits. On 64-bit systems, could passing physical or dma addresses
above 4gb instruct the firmware to export or import data to the wrong
physical memory area, leading to memory corruption?

[Severity: Critical]
This isn't a bug introduced by this patch, but regarding physical addresses,
is there an uninitialized address used for imem export/import?

In se_if_probe(), a buffer for the encrypted imem is allocated via
dmam_alloc_coherent(), which assigns the generated dma address to a local
variable imem_dma_addr. However, it seems this address is never stored back
into the load_fw->imem.phyaddr struct field, leaving it zero.

During suspend/resume, could se_save_imem_state() pass this zero address to
ele_service_swap(), instructing the secure enclave firmware to overwrite
physical address 0x0 with its state?

>  	struct se_api_msg *tx_msg __free(kfree)	=
>  		kzalloc(ELE_FW_AUTH_REQ_SZ, GFP_KERNEL);

[ ... ]

> @@ -224,8 +229,8 @@ int ele_fw_authenticate(struct se_if_priv *priv, phys_addr_t contnr_addr,
>  		return ret;
>  
>  	tx_msg->data[0] = lower_32_bits(contnr_addr);
> -	tx_msg->data[1] = upper_32_bits(contnr_addr);
> -	tx_msg->data[2] = img_addr;
> +	tx_msg->data[1] = 0;
> +	tx_msg->data[2] = lower_32_bits(img_addr);
>  
>  	ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_FW_AUTH_REQ_SZ, rx_msg,
>  			       ELE_FW_AUTH_RSP_MSG_SZ);

[Severity: Critical]
This is a pre-existing issue, but since ele_msg_send_rcv() is called here,
could dangling pointers left in the global callback handle on error paths
lead to use-after-free heap corruption or a null pointer dereference?

In ele_msg_send_rcv(), rx_msg and dev_ctx are stored in
priv->waiting_rsp_clbk_hdl. If ele_msg_send() fails, the function returns
without clearing them, and the caller frees the heap-allocated rx_msg.

If a delayed or spurious response arrives later, could the asynchronous
se_if_rx_callback() use the stale handle to blindly copy payload data into
the freed rx_msg buffer?

Furthermore, if ele_msg_rcv() is interrupted by a signal, it sets dev_ctx to
null but leaves rx_msg, which could lead se_if_rx_callback() to dereference
a null pointer when accessing se_clbk_hdl->dev_ctx->devname.

[Severity: High]
This is another pre-existing issue, but does ele_msg_rcv() have the
potential to cause an infinite busy loop if the calling task receives a
signal?

It uses wait_for_completion_interruptible_timeout(), which returns
-ERESTARTSYS immediately if a signal is pending. In this case, the code
sets signal_rcvd = true and uses continue to restart the loop without
clearing the signal or returning.

Since the signal remains pending, the next call to the wait function returns
-ERESTARTSYS immediately again without sleeping. If the hardware never
responds, could this indefinitely hang the cpu core?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=2

[PATCH 3/7] firmware: imx: ele: Bypass memcpy when ele_get_info() fails

[Peng Fan (OSS)]

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

When ele_get_info() fails, no sense to copy get_info_data to destination,
because get_info_data may contains garbage data, so bypass the copy.

Fixes: 106ffe5d78ad8 ("firmware: imx: add driver for NXP EdgeLock Enclave")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/ele_base_msg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
index f6346f15450963b9dd8d2df7d3e6b2ce6b4602ce..05a518df38cd6a068266757969b361249ef0560a 100644
--- a/drivers/firmware/imx/ele_base_msg.c
+++ b/drivers/firmware/imx/ele_base_msg.c
@@ -95,6 +95,8 @@ int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
 
 	ret = se_val_rsp_hdr_n_status(priv, rx_msg, ELE_GET_INFO_REQ,
 				      ELE_GET_INFO_RSP_MSG_SZ, true);
+	if (ret < 0)
+		goto exit;
 
 	memcpy(s_info, get_info_data, sizeof(*s_info));
 exit:

-- 
2.37.1

Re: [PATCH 3/7] firmware: imx: ele: Bypass memcpy when ele_get_info() fails

[sashiko-bot]

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

Pre-existing issues:
- [Critical] `load_fw->imem.phyaddr` is never updated with the DMA address returned by `dmam_alloc_coherent()`. It remains 0, causing the firmware to write 64KB to physical address 0 on suspend, destroying system memory.
- [High] In error and timeout paths of `ele_msg_send_rcv()`, the driver fails to safely clean up the callback state, leaving dangling pointers in `priv->waiting_rsp_clbk_hdl`. This leads to a Use-After-Free or NULL pointer dereference in the mailbox RX interrupt handler (`se_if_rx_callback`).
- [High] `ele_service_swap()` silently truncates the 64-bit `phys_addr_t` parameter to 32 bits, which will cause memory corruption if the allocated buffer resides above 4GB.
--

commit 268976f0cb80c396ef876e24ed7b353994d79e47
Author: Peng Fan <peng.fan@nxp.com>

firmware: imx: ele: Bypass memcpy when ele_get_info() fails

When ele_get_info() fails, no sense to copy get_info_data to destination,
because get_info_data may contains garbage data, so bypass the copy.

> diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
> index f6346f1545096..05a518df38cd6 100644
> --- a/drivers/firmware/imx/ele_base_msg.c
> +++ b/drivers/firmware/imx/ele_base_msg.c
> @@ -95,6 +95,8 @@ int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
>  
>  	ret = se_val_rsp_hdr_n_status(priv, rx_msg, ELE_GET_INFO_REQ,
>  				      ELE_GET_INFO_RSP_MSG_SZ, true);
> +	if (ret < 0)
> +		goto exit;
>  
>  	memcpy(s_info, get_info_data, sizeof(*s_info));
>  exit:

[Severity: High]
This isn't a bug introduced by this patch, but doesn't ele_msg_send_rcv()
fail to safely clean up the callback state on error paths?

In drivers/firmware/imx/ele_common.c:

int ele_msg_send_rcv(struct se_if_device_ctx *dev_ctx, void *tx_msg,
                     int tx_msg_sz, void *rx_msg, int exp_rx_msg_sz)
{
    ...
	priv->waiting_rsp_clbk_hdl.dev_ctx = dev_ctx;
	priv->waiting_rsp_clbk_hdl.rx_msg_sz = exp_rx_msg_sz;
	priv->waiting_rsp_clbk_hdl.rx_msg = rx_msg;

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

If ele_msg_send() fails, it returns early but leaves
priv->waiting_rsp_clbk_hdl.rx_msg pointing to rx_msg, which is often a local
heap-allocated buffer freed upon return.

Also, if ele_msg_rcv() times out, waiting_rsp_clbk_hdl.dev_ctx is set to NULL
but rx_msg is not cleared.

Could a delayed mailbox interrupt triggering se_if_rx_callback() result in a
use-after-free if memcpy() copies firmware data into this freed pointer?


[Severity: Critical]
This is a pre-existing issue, but does se_if_probe() neglect to save the DMA
address returned by dmam_alloc_coherent()?

In drivers/firmware/imx/se_ctrl.c:

static int se_if_probe(struct platform_device *pdev)
{
    ...
		if (load_fw->se_fw_img_nm->prim_fw_nm_in_rfs) {
			/* allocate buffer where SE store encrypted IMEM */
			imem_dma_addr = phys_to_dma(priv->dev, load_fw->imem.phyaddr);
			load_fw->imem.buf = dmam_alloc_coherent(priv->dev, ELE_IMEM_SIZE,
								&imem_dma_addr, GFP_KERNEL);

It seems load_fw->imem.phyaddr is never updated with imem_dma_addr. Since
load_fw points to a zero-initialized static variable, phyaddr remains 0.

During suspend, se_suspend() calls se_save_imem_state() which passes
imem->phyaddr (0) to ele_service_swap(). Does this cause the secure enclave
to export 64KB of IMEM data to physical address 0, corrupting system memory?


[Severity: High]
This is a pre-existing issue, but does ele_service_swap() silently truncate
the phys_addr_t parameter to 32 bits?

In drivers/firmware/imx/ele_base_msg.c:

int ele_service_swap(struct se_if_priv *priv,
		     phys_addr_t addr,
		     u32 addr_size, u16 flag)
{
    ...
	tx_msg->data[3] = lower_32_bits(addr);

Unlike ele_fw_authenticate(), this function doesn't seem to check if the
address exceeds 32 bits using upper_32_bits().

If dmam_alloc_coherent() provides a buffer above 4GB, will the firmware
perform DMA to the truncated 32-bit address and overwrite arbitrary memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=3

[PATCH 4/7] firmware: imx: ele: simplify SoC device registration

[Peng Fan (OSS)]

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

'soc_register' is used as a flag to control whether a SoC device should
be registered. However, only i.MX8ULP requires this, while i.MX9 is
handled separately in soc-imx9.c.

Replace the boolean 'soc_register' with a 'soc_name' string and use its
presence as the condition for registration to remove the switch and
i.MX93 case.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/se_ctrl.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
index 9327d47e4312e7f39a5f4fc24dd73566af240a9f..30e97a604eca137f45de6c329081fba234642732 100644
--- a/drivers/firmware/imx/se_ctrl.c
+++ b/drivers/firmware/imx/se_ctrl.c
@@ -56,7 +56,7 @@ struct se_var_info {
 /* contains fixed information */
 struct se_soc_info {
 	const u16 soc_id;
-	const bool soc_register;
+	const char *soc_name;
 	const struct se_fw_img_name se_fw_img_nm;
 };
 
@@ -72,7 +72,7 @@ static struct se_var_info var_se_info;
 
 static struct se_soc_info se_imx8ulp_info = {
 	.soc_id = SOC_ID_OF_IMX8ULP,
-	.soc_register = true,
+	.soc_name = "i.MX8ULP",
 	.se_fw_img_nm = {
 		.prim_fw_nm_in_rfs = IMX_ELE_FW_DIR
 			"mx8ulpa2-ahab-container.img",
@@ -163,7 +163,7 @@ static int get_se_soc_info(struct se_if_priv *priv, const struct se_soc_info *se
 	var_se_info.soc_rev = s_info->d_info.soc_rev;
 	load_fw->imem.state = s_info->d_addn_info.imem_state;
 
-	if (!se_info->soc_register)
+	if (!se_info->soc_name)
 		return 0;
 
 	attr = devm_kzalloc(priv->dev, sizeof(*attr), GFP_KERNEL);
@@ -181,14 +181,7 @@ static int get_se_soc_info(struct se_if_priv *priv, const struct se_soc_info *se
 						FIELD_GET(DEV_GETINFO_MAJ_VER_MASK,
 							  var_se_info.soc_rev));
 
-	switch (se_info->soc_id) {
-	case SOC_ID_OF_IMX8ULP:
-		attr->soc_id = "i.MX8ULP";
-		break;
-	case SOC_ID_OF_IMX93:
-		attr->soc_id = "i.MX93";
-		break;
-	}
+	attr->soc_id = se_info->soc_name;
 
 	err = of_property_read_string(of_root, "model", &attr->machine);
 	if (err)

-- 
2.37.1

RE: [PATCH 4/7] firmware: imx: ele: simplify SoC device registration

[Pankaj Gupta]

> -----Original Message-----
> From: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> Sent: 25 May 2026 11:09
> To: Frank Li <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Frieder Schrempf <frieder.schrempf@kontron.de>;
> Pankaj Gupta <pankaj.gupta@nxp.com>
> Cc: imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Peng Fan <peng.fan@nxp.com>
> Subject: [PATCH 4/7] firmware: imx: ele: simplify SoC device registration
> 
> From: Peng Fan <peng.fan@nxp.com>
> 
> 'soc_register' is used as a flag to control whether a SoC device should be
> registered. However, only i.MX8ULP requires this, while i.MX9 is handled
> separately in soc-imx9.c.
> 
> Replace the boolean 'soc_register' with a 'soc_name' string and use its
> presence as the condition for registration to remove the switch and
> i.MX93 case.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/firmware/imx/se_ctrl.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> index
> 9327d47e4312e7f39a5f4fc24dd73566af240a9f..30e97a604eca137f45de6c
> 329081fba234642732 100644
> --- a/drivers/firmware/imx/se_ctrl.c
> +++ b/drivers/firmware/imx/se_ctrl.c
> @@ -56,7 +56,7 @@ struct se_var_info {
>  /* contains fixed information */
>  struct se_soc_info {
>  	const u16 soc_id;
> -	const bool soc_register;
> +	const char *soc_name;
>  	const struct se_fw_img_name se_fw_img_nm;  };
> 
> @@ -72,7 +72,7 @@ static struct se_var_info var_se_info;
> 
>  static struct se_soc_info se_imx8ulp_info = {
>  	.soc_id = SOC_ID_OF_IMX8ULP,
> -	.soc_register = true,
> +	.soc_name = "i.MX8ULP",
>  	.se_fw_img_nm = {
>  		.prim_fw_nm_in_rfs = IMX_ELE_FW_DIR
>  			"mx8ulpa2-ahab-container.img",
> @@ -163,7 +163,7 @@ static int get_se_soc_info(struct se_if_priv *priv,
> const struct se_soc_info *se
>  	var_se_info.soc_rev = s_info->d_info.soc_rev;
>  	load_fw->imem.state = s_info->d_addn_info.imem_state;
> 
> -	if (!se_info->soc_register)
> +	if (!se_info->soc_name)
>  		return 0;

Remove the soc_register, all together.
Replace the check with "se_info->soc_id != SOC_ID_OF_IMX8ULP"

> 
>  	attr = devm_kzalloc(priv->dev, sizeof(*attr), GFP_KERNEL); @@ -
> 181,14 +181,7 @@ static int get_se_soc_info(struct se_if_priv *priv, const
> struct se_soc_info *se
> 
> 	FIELD_GET(DEV_GETINFO_MAJ_VER_MASK,
> 
> var_se_info.soc_rev));
> 
> -	switch (se_info->soc_id) {
> -	case SOC_ID_OF_IMX8ULP:
> -		attr->soc_id = "i.MX8ULP";
> -		break;
> -	case SOC_ID_OF_IMX93:
> -		attr->soc_id = "i.MX93";
> -		break;
> -	}
> +	attr->soc_id = se_info->soc_name;
> 
>  	err = of_property_read_string(of_root, "model", &attr->machine);
>  	if (err)
> 
> --
> 2.37.1

Re: [PATCH 4/7] firmware: imx: ele: simplify SoC device registration

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free and resource leak due to missing soc_device_unregister().
- [High] Information leak and functional breakage in get_se_soc_id() due to incorrect struct casting.
--

commit e7ad33f52996b1ca324a005406dedb896f547398
Author: Peng Fan <peng.fan@nxp.com>

firmware: imx: ele: simplify SoC device registration

This patch simplifies the SoC device registration for i.MX by replacing
the boolean 'soc_register' flag with a 'soc_name' string. The presence
of 'soc_name' now dictates registration, removing the need for a switch
statement.

> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> --- a/drivers/firmware/imx/se_ctrl.c
> +++ b/drivers/firmware/imx/se_ctrl.c
> @@ -56,7 +56,7 @@ struct se_var_info {
>  /* contains fixed information */
>  struct se_soc_info {
>  	const u16 soc_id;
> -	const bool soc_register;
> +	const char *soc_name;
>  	const struct se_fw_img_name se_fw_img_nm;
>  };

[Severity: High]
This isn't a bug introduced by this patch, but does get_se_soc_id()
incorrectly cast the match data and leak kernel address information?

device_get_match_data() returns a pointer to struct se_if_node, but
get_se_soc_id() assigns it to a const struct se_soc_info * pointer.

Since the first member of struct se_if_node is a pointer (se_info)
rather than a u16 soc_id, will accessing se_info->soc_id read the lower
16 bits of the kernel pointer instead of the actual SoC ID?

[ ... ]
> @@ -181,14 +181,7 @@ static int get_se_soc_info(struct se_if_priv *priv, const struct se_soc_info *se
>  						FIELD_GET(DEV_GETINFO_MAJ_VER_MASK,
>  							  var_se_info.soc_rev));
>  
> -	switch (se_info->soc_id) {
> -	case SOC_ID_OF_IMX8ULP:
> -		attr->soc_id = "i.MX8ULP";
> -		break;
> -	case SOC_ID_OF_IMX93:
> -		attr->soc_id = "i.MX93";
> -		break;
> -	}
> +	attr->soc_id = se_info->soc_name;
>  
>  	err = of_property_read_string(of_root, "model", &attr->machine);
>  	if (err)

[Severity: High]
This is a pre-existing issue, but does this function leak the soc_device
and create a use-after-free vulnerability?

The attr structure is allocated with devm_kzalloc(), meaning it is freed
when the driver unbinds. However, when soc_device_register(attr) is called
later in this function, the returned struct soc_device * is discarded.

Without calling soc_device_unregister() in error paths or driver remove,
will the sysfs nodes remain active? If a read to an attribute like
/sys/devices/soc0/revision happens after the driver is unbound, will it
dereference the freed attr memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=4

[PATCH 5/7] firmware: imx: ele: Correct check_hdr_exception_for_sz

[Peng Fan (OSS)]

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

header.size is u8 type, "header.size >= 0" will always return true.
Actually the check condition should be ">= 2", because a valid response
includes at lease header(4bytes) and status(4bytes), total 2 ints.

And ELE_DEBUG_DUMP_RSP_SZ is counted using bytes, need to divide it by 4.

Fixes: 106ffe5d78ad8 ("firmware: imx: add driver for NXP EdgeLock Enclave")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/ele_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
index 8daf32eded43c62daf56540b63e292bf0c6c9845..76bd3841acfcb1ff33d3bc5d900300493190b8a8 100644
--- a/drivers/firmware/imx/ele_common.c
+++ b/drivers/firmware/imx/ele_common.c
@@ -164,7 +164,7 @@ static bool check_hdr_exception_for_sz(struct se_if_priv *priv,
 	 */
 	if (header->command == ELE_DEBUG_DUMP_REQ &&
 	    header->ver == priv->if_defs->base_api_ver &&
-	    header->size >= 0 && header->size <= ELE_DEBUG_DUMP_RSP_SZ)
+	    header->size >= 2 && header->size <= (ELE_DEBUG_DUMP_RSP_SZ / 4))
 		return true;
 
 	return false;

-- 
2.37.1

Re: [PATCH 5/7] firmware: imx: ele: Correct check_hdr_exception_for_sz

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Use-After-Free in `ele_msg_send_rcv()` if `ele_msg_send()` fails.
- [Critical] Unconditional NULL Pointer Dereference in `se_if_rx_callback()` on unsolicited firmware responses.
- [Critical] Heap Buffer Overflow in NVM command handler inside `se_if_rx_callback()`.
- [Critical] Direct Dereference of User Space Pointers without `copy_from_user()` violating SMAP.
- [High] Unbounded Memory Allocation in `SE_IOCTL_CMD_SEND_RCV_RSP` via `cmd_snd_rcv_rsp_info.rx_buf_sz`.
- [High] Heap Out-of-Bounds Read and Information Leak due to exception-handling bypass in `se_if_rx_callback()`.
--

commit b7cffebfaeed965de6c2c0dd99ae2c4e1cc48a37
Author: Peng Fan <peng.fan@nxp.com>

firmware: imx: ele: Correct check_hdr_exception_for_sz
    
header.size is u8 type, "header.size >= 0" will always return true.
Actually the check condition should be ">= 2", because a valid response
includes at lease header(4bytes) and status(4bytes), total 2 ints.

And ELE_DEBUG_DUMP_RSP_SZ is counted using bytes, need to divide it by 4.

Fixes: 106ffe5d78ad8 ("firmware: imx: add driver for NXP EdgeLock Enclave")
Signed-off-by: Peng Fan <peng.fan@nxp.com>

> diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> index 8daf32eded43c..76bd3841acfcb 100644
> --- a/drivers/firmware/imx/ele_common.c
> +++ b/drivers/firmware/imx/ele_common.c
> @@ -164,7 +164,7 @@ static bool check_hdr_exception_for_sz(struct se_if_priv *priv,
>  	 */
>  	if (header->command == ELE_DEBUG_DUMP_REQ &&
>  	    header->ver == priv->if_defs->base_api_ver &&
> -	    header->size >= 0 && header->size <= ELE_DEBUG_DUMP_RSP_SZ)
> +	    header->size >= 2 && header->size <= (ELE_DEBUG_DUMP_RSP_SZ / 4))
>  		return true;
>  
>  	return false;

[Severity: High]
This is a pre-existing issue, but I noticed something concerning in how
check_hdr_exception_for_sz() is used. In se_if_rx_callback(), if this
function returns true, it skips the size constraint check:

drivers/firmware/imx/ele_common.c:se_if_rx_callback() {
    [ ... ]
		if (rx_msg_sz != se_clbk_hdl->rx_msg_sz &&
		    !check_hdr_exception_for_sz(priv, header)) {
            [ ... ]
			se_clbk_hdl->rx_msg_sz = min(rx_msg_sz, se_clbk_hdl->rx_msg_sz);
		}
	}
    [ ... ]
	memcpy(se_clbk_hdl->rx_msg, msg, se_clbk_hdl->rx_msg_sz);
    [ ... ]
}

Does this bypass allow se_clbk_hdl->rx_msg_sz to retain an unvalidated,
potentially large user-provided value? If so, couldn't the memcpy()
read past the bounds of the hardware mailbox msg buffer and leak adjacent
kernel memory to userspace?

[Severity: Critical]
This is a pre-existing issue, but while looking at se_if_rx_callback(),
I also noticed what looks like an unconditional overwrite for NVM commands:

drivers/firmware/imx/ele_common.c:se_if_rx_callback() {
    [ ... ]
		if (rx_msg_sz > MAX_NVM_MSG_LEN) {
            [ ... ]
			se_clbk_hdl->rx_msg_sz = MAX_NVM_MSG_LEN;
		}
		se_clbk_hdl->rx_msg_sz = rx_msg_sz;
    [ ... ]
}

Does the assignment se_clbk_hdl->rx_msg_sz = rx_msg_sz entirely defeat the
MAX_NVM_MSG_LEN check? Since the rx_msg buffer is pre-allocated with
MAX_NVM_MSG_LEN, could a large payload from the firmware cause a heap
buffer overflow in the subsequent memcpy()?

[Severity: Critical]
This isn't a bug introduced by this patch, but there might be a NULL pointer
dereference in se_if_rx_callback() when receiving unsolicited messages.

drivers/firmware/imx/ele_common.c:se_if_rx_callback() {
    [ ... ]
	if (header->tag == priv->if_defs->cmd_tag) {
		se_clbk_hdl = &priv->cmd_receiver_clbk_hdl;
		dev_dbg(dev, "Selecting cmd receiver:%s for mesg header:0x%x.",
			se_clbk_hdl->dev_ctx->devname,  *(u32 *)header);
    [ ... ]
}

If the driver is idle, won't se_clbk_hdl->dev_ctx be NULL? It looks like
dereferencing dev_ctx->devname here would cause an Oops if the firmware sends
an unexpected response or command.

[Severity: Critical]
This is a pre-existing issue, but I have a question about the ioctl handler
se_ioctl_cmd_snd_rcv_rsp_handler() in se_ctrl.c:

drivers/firmware/imx/se_ctrl.c:se_ioctl_cmd_snd_rcv_rsp_handler() {
    [ ... ]
	err = se_chk_tx_msg_hdr(priv, (struct se_msg_hdr *)cmd_snd_rcv_rsp_info.tx_buf);
    [ ... ]
	struct se_api_msg *tx_msg __free(kfree) =
		memdup_user(cmd_snd_rcv_rsp_info.tx_buf,
			    cmd_snd_rcv_rsp_info.tx_buf_sz);
    [ ... ]
}

Since cmd_snd_rcv_rsp_info.tx_buf is a user-space pointer, is it safe to
cast it and pass it to se_chk_tx_msg_hdr() before memdup_user() is called?
Wouldn't direct dereferencing of this pointer inside se_chk_tx_msg_hdr()
trigger an immediate panic on systems with SMAP enabled, or a page fault if
the pointer is invalid?

[Severity: High]
This isn't a bug introduced by this patch, but I spotted a potential unbounded
allocation in the same ioctl handler:

drivers/firmware/imx/se_ctrl.c:se_ioctl_cmd_snd_rcv_rsp_handler() {
    [ ... ]
	struct se_api_msg *rx_msg __free(kfree) =
		kzalloc(cmd_snd_rcv_rsp_info.rx_buf_sz, GFP_KERNEL);
    [ ... ]
}

Does this code safely handle large or negative values for rx_buf_sz?
Since the size comes directly from the user, could a negative value wrap
to a massive size_t and trigger a kernel warning or memory exhaustion when
passed to kzalloc()?

[Severity: Critical]
This is a pre-existing issue, but there seems to be a possible use-after-free
involving ele_msg_send_rcv() and se_ioctl_cmd_snd_rcv_rsp_handler():

drivers/firmware/imx/ele_common.c:ele_msg_send_rcv() {
    [ ... ]
	priv->waiting_rsp_clbk_hdl.dev_ctx = dev_ctx;
	priv->waiting_rsp_clbk_hdl.rx_msg_sz = exp_rx_msg_sz;
	priv->waiting_rsp_clbk_hdl.rx_msg = rx_msg;

	err = ele_msg_send(dev_ctx, tx_msg, tx_msg_sz);
	if (err < 0)
		return err;
    [ ... ]
}

If ele_msg_send() fails, ele_msg_send_rcv() returns immediately while
priv->waiting_rsp_clbk_hdl.dev_ctx remains non-NULL.

Back in se_ioctl_cmd_snd_rcv_rsp_handler():

drivers/firmware/imx/se_ctrl.c:se_ioctl_cmd_snd_rcv_rsp_handler() {
    [ ... ]
	struct se_api_msg *rx_msg __free(kfree) =
		kzalloc(cmd_snd_rcv_rsp_info.rx_buf_sz, GFP_KERNEL);
    [ ... ]
	err = ele_msg_send_rcv(dev_ctx, tx_msg, cmd_snd_rcv_rsp_info.tx_buf_sz,
			       rx_msg, cmd_snd_rcv_rsp_info.rx_buf_sz);
    [ ... ]
}

When the function returns, rx_msg is automatically freed. If a delayed
response later arrives from the firmware, wouldn't se_if_rx_callback()
see a valid dev_ctx and copy the payload into the already-freed rx_msg
pointer, causing a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=5

[PATCH 6/7] firmware: imx: ele: Use dev_err for error report

[Peng Fan (OSS)]

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

Use dev_err() to replace dev_dbg() for error report.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/ele_base_msg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
index 05a518df38cd6a068266757969b361249ef0560a..2f0046cd5ad06d28c11ad5bf7b8544d1aa9b9bb6 100644
--- a/drivers/firmware/imx/ele_base_msg.c
+++ b/drivers/firmware/imx/ele_base_msg.c
@@ -74,7 +74,7 @@ int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
 						   &get_info_addr,
 						   GFP_KERNEL);
 	if (!get_info_data) {
-		dev_dbg(priv->dev,
+		dev_err(priv->dev,
 			"%s: Failed to allocate get_info_addr.", __func__);
 		return -ENOMEM;
 	}

-- 
2.37.1

Re: [PATCH 6/7] firmware: imx: ele: Use dev_err for error report

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The introduced dev_err() lacks a trailing newline, contains a typo, and introduces redundant OOM logging.
--

commit c914c76ccf6cf2fdb05859267c584040fec1d7b6
Author: Peng Fan <peng.fan@nxp.com>

firmware: imx: ele: Use dev_err for error report

This commit replaces a dev_dbg() call with dev_err() for reporting a
memory allocation failure in ele_get_info().

> diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
> index 05a518df38cd6..2f0046cd5ad06 100644
> --- a/drivers/firmware/imx/ele_base_msg.c
> +++ b/drivers/firmware/imx/ele_base_msg.c
> @@ -74,7 +74,7 @@ int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
>  						   &get_info_addr,
>  						   GFP_KERNEL);
>  	if (!get_info_data) {
> -		dev_dbg(priv->dev,
> +		dev_err(priv->dev,
>  			"%s: Failed to allocate get_info_addr.", __func__);

[Severity: Low]
Is it possible to add a trailing newline (\n) to this error message?
Without it, the kernel log output could be mangled or concatenated with
subsequent messages.

Also, does this message contain a typo? It mentions failing to allocate
get_info_addr, but the actual buffer being allocated is get_info_data.

Finally, since dma_alloc_coherent() already natively dumps a stack trace on
allocation failure, is this custom out-of-memory error message partially
redundant? While these string contents existed prior to this patch, promoting
it to dev_err() makes these formatting and style issues unconditionally
visible.

>  		return -ENOMEM;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=6

[PATCH 7/7] firmware: imx: ele: Fix debug dump size handling

[Peng Fan (OSS)]

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

The ELE debug dump response is formatted as:

  - header (1 u32)
  - status (1 u32)
  - dump words (2..21 u32)
  - CRC (1 u32)

The header.size field represents the total number of u32 words,
not a byte count.

After removing the header and status (2 words), header.size still
includes the trailing CRC. Therefore, when determining the number
of valid debug words, the CRC must be excluded as well.

The existing check:

  if (rx_msg->header.size > 4)

is incorrect because header.size has already been reduced by 2.
Fix it by comparing against the remaining minimum (CRC + debug words),
i.e.:

  if (rx_msg->header.size > 2)

and then decrement to account for the CRC.

Fixes: 106ffe5d78ad8 ("firmware: imx: add driver for NXP EdgeLock Enclave")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/ele_base_msg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
index 2f0046cd5ad06d28c11ad5bf7b8544d1aa9b9bb6..ec718d322abcd7e120d916bbcbcf691f3078c628 100644
--- a/drivers/firmware/imx/ele_base_msg.c
+++ b/drivers/firmware/imx/ele_base_msg.c
@@ -290,7 +290,7 @@ int ele_debug_dump(struct se_if_priv *priv)
 
 		rx_msg->header.size -= 2;
 
-		if (rx_msg->header.size > 4)
+		if (rx_msg->header.size > 2)
 			rx_msg->header.size--;
 
 		for (i = 0; i < rx_msg->header.size; i += 2)

-- 
2.37.1

Re: [PATCH 7/7] firmware: imx: ele: Fix debug dump size handling

[sashiko-bot]

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

New issues:
- [Medium] The patch fails to correctly exclude the CRC from being printed when the number of debug dump words is odd, resulting in an incomplete fix.

Pre-existing issues:
- [Critical] A Use-After-Free (UAF) and NULL pointer dereference vulnerability exists in `se_if_rx_callback` due to improper cleanup after a command timeout.
--

commit 71d8abdcd91f20629227af0e25e2702c3ad5260f
Author: Peng Fan <peng.fan@nxp.com>

firmware: imx: ele: Fix debug dump size handling

This commit attempts to fix the ELE debug dump response handling by excluding
the trailing CRC from the valid debug words printed. It updates the size
threshold condition and decrements the total size.

> diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
> index 2f0046cd5ad06..ec718d322abcd 100644
> --- a/drivers/firmware/imx/ele_base_msg.c
> +++ b/drivers/firmware/imx/ele_base_msg.c
> @@ -290,7 +290,7 @@ int ele_debug_dump(struct se_if_priv *priv)

[Severity: Critical]
This is a pre-existing issue, but can a timeout in ele_msg_send_rcv() lead to
a use-after-free and NULL pointer dereference if a late response arrives?

If ele_msg_send_rcv() times out earlier in ele_debug_dump(), it returns
-ETIMEDOUT, causing the function to exit and free the locally allocated
rx_msg buffer.

Looking at ele_msg_send_rcv() on a timeout path, it sets
priv->waiting_rsp_clbk_hdl.dev_ctx = NULL but appears to leave the
priv->waiting_rsp_clbk_hdl.rx_msg pointer intact.

If a late response subsequently arrives with matching tags,
se_if_rx_callback() will process it:

se_if_rx_callback() {
    ...
    dev_dbg(se_clbk_hdl->dev_ctx->devname, ...);
    ...
    memcpy(se_clbk_hdl->rx_msg, msg, se_clbk_hdl->rx_msg_sz);
}

Would this result in dereferencing the NULL dev_ctx and copying the payload
into the freed rx_msg buffer?

>  
>  		rx_msg->header.size -= 2;
>  
> -		if (rx_msg->header.size > 4)
> +		if (rx_msg->header.size > 2)
>  			rx_msg->header.size--;
>  
>  		for (i = 0; i < rx_msg->header.size; i += 2)

[Severity: Medium]
Does this fully prevent the CRC from being printed if the firmware returns an
odd number of debug words?

The loop increments by 2 and unconditionally prints both data[i + 1] and
data[i + 2].

If the firmware returns 3 debug words and 1 CRC, rx_msg->header.size
evaluates to 3 after the decrement. The loop will execute for i = 2
(since 2 < 3) and print both data[3] and data[4], which means the CRC at
data[4] is still printed.

Additionally, does the rx_msg->header.size > 2 condition incorrectly skip
decrementing the size if the response contains 0 or 1 debug words?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-ele-v1-v1-0-a9570c4bffc9@nxp.com?part=7

Re: [PATCH 0/7] firmware: imx: ele: misc fixes

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>


On Mon, 25 May 2026 13:39:23 +0800, Peng Fan (OSS) wrote:
> Misc fixes when reading the ele driver, including check condition, size
> mismatch and etc. More information could be found in each patch commit
> log.

Applied, thanks!

[1/7] firmware: imx: ele: Correct check condition in se_if_rx_callback
      commit: affeaaa41902fd1a6725c1d0cf3bdd32d78473c4
[2/7] firmware: imx: ele: Correct ele_fw_authenticate API
      commit: a3e16ecfa7a19205323ca2b055aaaaba8d375320
[3/7] firmware: imx: ele: Bypass memcpy when ele_get_info() fails
      commit: dbbf3f2e2806b910fab25d44db892dce81daa45d
[4/7] firmware: imx: ele: simplify SoC device registration
      commit: 1c140aa865f91ed8e6b2d5105adae81742b41971
[5/7] firmware: imx: ele: Correct check_hdr_exception_for_sz
      commit: 1065132b2bb3de722c44b1181e2b613238f62a9d
[6/7] firmware: imx: ele: Use dev_err for error report
      commit: c0b4f6fbaa3d36824718f10ccc0d7b3cf765eb32
[7/7] firmware: imx: ele: Fix debug dump size handling
      commit: 406d072598536b03078dbccf54009a2faa3d6108

squash to previous patches.

Frank

Best regards,
--
Frank Li <Frank.Li@nxp.com>