* Re: [PATCH -next] firmware: imx: se_ctrl: detect round_up() overflow in iobuf setup
2026-06-11 15:41 [PATCH -next] firmware: imx: se_ctrl: detect round_up() overflow in iobuf setup pankaj.gupta
@ 2026-06-11 10:37 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-11 10:37 UTC (permalink / raw)
To: pankaj.gupta; +Cc: imx, Frank.Li
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
^ permalink raw reply [flat|nested] 2+ messages in thread