Re: [PATCH v36 5/7] firmware: imx: adds miscdev

[Frank Li <Frank.li@oss.nxp.com>]

On Tue, Aug 18, 2026 at 09:04:59AM +0000, Pankaj Gupta (OSS) wrote:
> > > > > --- a/drivers/firmware/imx/ele_common.c
> > > > > +++ b/drivers/firmware/imx/ele_common.c
> > > > > @@ -101,15 +101,15 @@ int se_val_cmd_addrs(struct se_if_device_ctx
> > > > > *dev_ctx, struct se_api_msg *msg,
> > > > >
> > > > >             addr = msg->data[f->lsb_idx];
> > > > >             if (f->has_msb)
> > > > >                     addr |= (u64)msg->data[f->msb_idx] << 32;
> > > > >
> > > > >             /* Zero marks an absent optional buffer. */
> > > > >             if (!addr)
> > > > >                     continue;
> > > > >
> > > > >             if (addr < base || addr >= end)
> > > > >                     return -EACCES;
> > > > >
> > > > >             /*
> > > > >              * When the message also carries this buffer's length,
> > > > > the
> > > > whole
> > > > >              * buffer [addr, addr + len) must fit inside the
> > > > > window, not
> > > > just
> > > >
> > > > [Severity: High]
> > > > Does this code prevent firmware from reading outside the shared
> > > > memory window when the command lacks an explicit buffer size?
> > > >
> > > > For commands like ELE_OEM_AUTH_CONTAINER_REQ that use
> > > > SE_CMD_ADDR_NO_SIZE with no buf_size, only the start address is
> > > > checked against the bounds. If a caller provides a start address at
> > > > the very end of the window (e.g., end - 1), could the firmware read
> > > > the payload out-of-bounds into adjacent physical memory?
> > >
> > > - Not a bug - by design.
> > > - The existing comment explicitly documents the rationale:
> > >    -- this is a read-only input buffer
> > >        --- (firmware copies the container header into its own internal memory
> > for authentication; no DMA write-back through this address).
> > >    -- A rogue caller can at most cause firmware to read within the shared-
> > memory window it already owns.
> > > - Output buffers must be fully bounded;
> > > - Input-only buffers are safe with addr-only checks.
> >
> > Why? at lease addr should include payload size?
> >
> > Frank
>
> Payload size is encoded inside the OEM Authentication Container itself.
> For ELE_OEM_AUTH_CONTAINER_REQ, the API only passes the start address of the container to firmware.
>
> The kernel cannot validate the full container size because determining the actual payload size requires parsing the container header, and that header is interpreted by firmware using information and keys that are not available outside the secure firmware.

please wrap you reply below 80/100 chars each line.

>
> Therefore, for this command, only the container start address is validated against the shared-memory region.
> The firmware then reads and processes the container contents as part of the authentication flow.
>
> Also note that this is a read-only input buffer.
> Firmware does not perform any write-back through this address.
> A malicious caller can only provide invalid input data for authentication,
> which would cause the authentication operation to fail.
> It does not provide a mechanism for arbitrary memory corruption or privilege escalation from the kernel side.
>
> The trust model for this command assumes that the firmware is responsible for validating and parsing the container format after receiving the start address. Since the container length is not available to the kernel, enforcing an addr + size check in the driver is not possible for this request type. Any bounds validation based on the authenticated container structure must therefore be performed by the firmware itself.

You can provide comments to explain situation.

Frank

>
> Regards
> Pankaj
>
> NXP Confidential