Re: [PATCH -next] firmware: imx: se_ctrl: detect round_up() overflow in iobuf setup

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Returning -EBUSY from the file release handler on signal interruption causes permanent memory and resource leaks.
- [High] Concurrent modification of shared linked list `priv->dev_ctx_list` under a per-instance lock leads to list corruption.
- [High] Use-After-Free of `dev_ctx->fops_lock` in `se_if_fops_close` upon scope exit.
- [High] Infinite kernel loop in `ele_msg_rcv` when interrupted by a pending signal.
- [High] Unprivileged `read()` operations corrupt the global command receiver state.
--

commit 274dde5deb429b8b8fa60b9158c528c3a6c4f3bc
Author: Pankaj Gupta <pankaj.gupta@nxp.com>

firmware: imx: se_ctrl: detect round_up() overflow in iobuf setup

se_ioctl_setup_iobuf_handler() aligns io.length with round_up(..., 8)
before checking the available shared memory space. On 32-bit builds,
round_up() can overflow for large io.length values and wrap aligned_len
to a smaller value, which can bypass the bounds check while later
memset() still uses the original unbounded io.length.

> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> index 8fab3b7767b7a..5b23485fe42c8 100644
> --- a/drivers/firmware/imx/se_ctrl.c
> +++ b/drivers/firmware/imx/se_ctrl.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but in se_if_fops_close(), could returning
-EBUSY on signal interruption cause memory and resource leaks?

drivers/firmware/imx/se_ctrl.c:se_if_fops_close() {
    ...
	scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
    ...

If the process closing the file has a pending signal, mutex_lock_interruptible
fails and returns -EINTR, causing scoped_cond_guard to immediately return
-EBUSY. 

The VFS layer ignores error returns from release handlers and will not retry.
This drops the file structure but permanently bypasses the cleanup block,
permanently leaking the dev_ctx memory and its associated shared DMA buffers. 

Additionally, if the leaked context was the designated command receiver,
priv->cmd_receiver_clbk_hdl.dev_ctx remains pointing to it forever,
preventing any other process from acquiring the command receiver role.


[Severity: High]
This isn't a bug introduced by this patch, but could this concurrent list
modification lead to corruption?

drivers/firmware/imx/se_ctrl.c:se_if_fops_close() {
    ...
	scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
        ...
		list_del(&dev_ctx->link);
        ...

These shared structure modifications are protected solely by dev_ctx->fops_lock,
which is a per-file-descriptor lock local to the current dev_ctx instance. 

If multiple processes close their file descriptors concurrently, each acquires
its own distinct fops_lock and they concurrently execute list_del on the
shared priv->dev_ctx_list.


[Severity: High]
This is a pre-existing issue, but does this scoped_cond_guard usage
trigger a use-after-free?

drivers/firmware/imx/se_ctrl.c:se_if_fops_close() {
    ...
	scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
        ...
		kfree(dev_ctx);
	}
    ...

Because dev_ctx is freed inside the guarded block, when the block ends,
the implicitly generated scope destructor executes
mutex_unlock(&dev_ctx->fops_lock).

This dereferences a freed pointer and writes into freed memory to update
the mutex state.


[Severity: High]
This isn't a bug introduced by this patch, but can an unprivileged read()
corrupt the global command receiver state?

drivers/firmware/imx/se_ctrl.c:se_if_fops_read() {
    ...
	scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
		if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx) {
			err = -EINVAL;
			goto exit;
		}
        ...
exit:
		priv->cmd_receiver_clbk_hdl.rx_msg_sz = 0;
        ...

If a process that is not the designated command receiver calls read(), it
gets -EINVAL and jumps to exit, unconditionally clearing the rx_msg_sz field
of the legitimate command receiver.

Could a malicious reader zero out the size concurrently while the legitimate
receiver is fetching a response, causing it to read a corrupted size of 0?


[Severity: High]
This is a pre-existing issue, but can ele_msg_rcv() get stuck in an
infinite loop?

drivers/firmware/imx/ele_common.c:ele_msg_rcv() {
    ...
	do {
        ...
		ret = wait_for_completion_interruptible_timeout(&se_clbk_hdl->done, timeout_ms);
		if (ret == -ERESTARTSYS) {
			if (dev_ctx->priv->waiting_rsp_clbk_hdl.dev_ctx) {
				dev_ctx->priv->waiting_rsp_clbk_hdl.signal_rcvd = true;
				continue;
			}
            ...
		}
        ...
	} while (ret < 0);
    ...

If wait_for_completion_interruptible_timeout() returns -ERESTARTSYS due to
a pending signal, the code sets signal_rcvd and executes continue.

Since the signal remains pending on the kernel thread, the next call to
wait_for_completion_interruptible_timeout() will instantly return
-ERESTARTSYS again, creating an infinite tight loop.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611154148.3182727-1-pankaj.gupta@oss.nxp.com?part=1