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

[sashiko-bot@kernel.org]

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