Re: [PATCH v51 1/7] Documentation/firmware: add imx/se to other_interfaces

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Artificially intercepting a successfully completed firmware command to return `-EINTR` leads to firmware resource leaks and violates standard syscall semantics.
--

commit 08f44ced154dcd881139680cedb3ca1074757a26
Author: Pankaj Gupta <pankaj.gupta@nxp.com>

    Documentation/firmware: add imx/se to other_interfaces
    
    Documents i.MX SoC's Service layer and C_DEV driver for selected SoC(s)
    that contains the NXP hardware IP(s) for Secure Enclaves(se) like:
    - NXP EdgeLock Enclave on i.MX93 & i.MX8ULP

> diff --git a/Documentation/driver-api/firmware/other_interfaces.rst b/Documentation/driver-api/firmware/other_interfaces.rst
> index 06ac89adaafba..984ee3ecc8dc2 100644
> --- a/Documentation/driver-api/firmware/other_interfaces.rst
> +++ b/Documentation/driver-api/firmware/other_interfaces.rst

[ ... ]

> +      |            [fw_api_specific_ops()         |
> +      |             (is_cmd_interrupted=true):    |
> +      |             for SESSION_OPEN: record      |
> +      |             handle, close session via     |
> +      |             se_close_session(), clear;    |
> +      |             for STORAGE_OPEN: record      |
> +      |             handle, close storage via     |
> +      |             se_close_storage(), return 0] |
> +      |            err = -EINTR                   |
> +      |            (not -ERESTARTSYS:             |
> +      |             prevents VFS auto-restart)    |
> +      |<--ioctl -EINTR-----|                      |
> +      |                    |                      |
> +      | [signal handler runs; userspace decides   |
> +      |  whether to re-issue; FW handle tracked   |
> +      |  or cleaned up; no firmware resource leak]|

[Severity: High]
Does this behavior leak firmware resources for commands other than SESSION_OPEN
and STORAGE_OPEN?

If a non-fatal signal is delivered during the ioctl wait for operations like
ELE_MAC_OPEN_REQ, ele_msg_send_rcv() overrides the successful result with
-ERESTARTSYS.

Then se_ioctl_cmd_snd_rcv_rsp_handler() catches this, calls
fw_api_specific_ops(is_cmd_interrupted=true), and returns -EINTR.

However, fw_api_specific_ops() only implements rollback for session and
storage handles:

ele_fw_api.c:fw_api_specific_ops() {
    ...
    switch (header->command) {
    case ELE_SESSION_OPEN_REQ:
        ...
    case ELE_STORAGE_OPEN_REQ:
        ...
    }
}

This silently ignores and leaks all other handle-creating operations.

Because -EINTR is returned, the response containing the handle is not copied
to userspace. This permanently leaks the resource in the secure enclave, and
could allow unprivileged users to cause a denial of service by sending
signals to their own process during ioctls.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-imx-se-if-v51-0-4a7dac612cb5@nxp.com?part=1