[PATCH -next] firmware: imx: secure-enclave: drop invalid overflow check in iobuf setup

[PATCH -next] firmware: imx: secure-enclave: drop invalid overflow check in iobuf setup

[Pankaj Gupta]

io.length is a 32-bit value, so comparing it against SIZE_MAX is
tautological and always false on 64-bit systems. Clang flags this as
a constant out-of-range comparison.

Remove the invalid check and rely on size_t-based alignment and the
existing shared memory bounds validation.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202605081441.lIHK75ug-lkp@intel.com/
Fixes: 890e531b38f89 ("firmware: drivers: imx: adds miscdev")
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
---
 drivers/firmware/imx/se_ctrl.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
index f88e581fc147..1683b259864f 100644
--- a/drivers/firmware/imx/se_ctrl.c
+++ b/drivers/firmware/imx/se_ctrl.c
@@ -670,11 +670,6 @@ static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
 		goto copy;
 	}
 
-	if (io.length > SIZE_MAX - 7) {
-		dev_err(dev_ctx->priv->dev, "%s: Invalid buffer length.",
-			dev_ctx->devname);
-		return -EINVAL;
-	}
 	aligned_len = round_up((size_t)io.length, 8);
 
 	/* No specific requirement for this buffer. */
-- 
2.43.0

Re: [PATCH -next] firmware: imx: secure-enclave: drop invalid overflow check in iobuf setup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Removing the `io.length > SIZE_MAX - 7` check introduces an integer overflow in `round_up()` on 32-bit architectures, leading to a massive out-of-bounds write.
--

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

firmware: imx: secure-enclave: drop invalid overflow check in iobuf setup

This commit removes a check comparing io.length against SIZE_MAX - 7, as it
always evaluates to false on 64-bit systems. It instead relies on size_t-based
alignment and existing shared memory bounds validation.

Closes: https://lore.kernel.org/oe-kbuild-all/202605081441.lIHK75ug-lkp@intel.com/

> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> index f88e581fc147e..1683b259864f1 100644
> --- a/drivers/firmware/imx/se_ctrl.c
> +++ b/drivers/firmware/imx/se_ctrl.c
> @@ -670,11 +670,6 @@ static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
>  		goto copy;
>  	}
>  
> -	if (io.length > SIZE_MAX - 7) {
> -		dev_err(dev_ctx->priv->dev, "%s: Invalid buffer length.",
> -			dev_ctx->devname);
> -		return -EINVAL;
> -	}
>  	aligned_len = round_up((size_t)io.length, 8);

On 32-bit platforms where size_t is 32 bits, can this round_up() overflow if
io.length is near U32_MAX?

If io.length is between 0xFFFFFFF9 and 0xFFFFFFFF, round_up() will wrap
around to 0.

If aligned_len wraps to 0, does it bypass the subsequent shared memory bounds
validation?

	if (shared_mem->size < shared_mem->pos ||
	    aligned_len > (shared_mem->size - shared_mem->pos)) {

This appears to cause a regression where the following memset() can perform an
out-of-bounds write using the original un-rounded io.length value:

	memset(shared_mem->ptr + pos, 0, io.length);

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