[PATCH] accel/ivpu: Add bounds check for firmware runtime memory

[PATCH] accel/ivpu: Add bounds check for firmware runtime memory

[Andrzej Kacprowski]

Validate that the firmware runtime memory specified in the image
header is properly aligned and sized to hold the firmware image.
This prevents errors during memory allocation and image transfer.

Fixes: 2007e210b6a1 ("accel/ivpu: Split FW runtime and global memory buffers")
Cc: <stable@vger.kernel.org> # v7.0+
Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_fw.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
index 107f8ad31050..33c50779c06b 100644
--- a/drivers/accel/ivpu/ivpu_fw.c
+++ b/drivers/accel/ivpu/ivpu_fw.c
@@ -259,6 +259,22 @@ static int ivpu_fw_parse(struct ivpu_device *vdev)
 		return -EINVAL;
 	}
 
+	if (!PAGE_ALIGNED(runtime_addr)) {
+		ivpu_err(vdev, "Runtime address 0x%llx not page aligned\n", runtime_addr);
+		return -EINVAL;
+	}
+
+	if (!PAGE_ALIGNED(runtime_size)) {
+		ivpu_err(vdev, "Runtime size %llu not page aligned\n", runtime_size);
+		return -EINVAL;
+	}
+
+	if (runtime_size < image_size) {
+		ivpu_err(vdev, "Runtime size too small: %llu, image size: %llu\n",
+			 runtime_size, image_size);
+		return -EINVAL;
+	}
+
 	if (!ivpu_is_within_range(image_load_addr, image_size, &vdev->hw->ranges.runtime)) {
 		ivpu_err(vdev, "Invalid firmware load address: 0x%llx and size %llu\n",
 			 image_load_addr, image_size);
-- 
2.43.0

Re: [PATCH] accel/ivpu: Add bounds check for firmware runtime memory

[Wachowski, Karol]

On 29-May-26 14:08, Andrzej Kacprowski wrote:
> Validate that the firmware runtime memory specified in the image
> header is properly aligned and sized to hold the firmware image.
> This prevents errors during memory allocation and image transfer.
> 
> Fixes: 2007e210b6a1 ("accel/ivpu: Split FW runtime and global memory buffers")
> Cc: <stable@vger.kernel.org> # v7.0+
> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>

Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>

> ---
>   drivers/accel/ivpu/ivpu_fw.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
> index 107f8ad31050..33c50779c06b 100644
> --- a/drivers/accel/ivpu/ivpu_fw.c
> +++ b/drivers/accel/ivpu/ivpu_fw.c
> @@ -259,6 +259,22 @@ static int ivpu_fw_parse(struct ivpu_device *vdev)
>   		return -EINVAL;
>   	}
>   
> +	if (!PAGE_ALIGNED(runtime_addr)) {
> +		ivpu_err(vdev, "Runtime address 0x%llx not page aligned\n", runtime_addr);
> +		return -EINVAL;
> +	}
> +
> +	if (!PAGE_ALIGNED(runtime_size)) {
> +		ivpu_err(vdev, "Runtime size %llu not page aligned\n", runtime_size);
> +		return -EINVAL;
> +	}
> +
> +	if (runtime_size < image_size) {
> +		ivpu_err(vdev, "Runtime size too small: %llu, image size: %llu\n",
> +			 runtime_size, image_size);
> +		return -EINVAL;
> +	}
> +
>   	if (!ivpu_is_within_range(image_load_addr, image_size, &vdev->hw->ranges.runtime)) {
>   		ivpu_err(vdev, "Invalid firmware load address: 0x%llx and size %llu\n",
>   			 image_load_addr, image_size);

Re: [PATCH] accel/ivpu: Add bounds check for firmware runtime memory

[Wachowski, Karol]

On 29-May-26 14:27, Wachowski, Karol wrote:
> On 29-May-26 14:08, Andrzej Kacprowski wrote:
>> Validate that the firmware runtime memory specified in the image
>> header is properly aligned and sized to hold the firmware image.
>> This prevents errors during memory allocation and image transfer.
>>
>> Fixes: 2007e210b6a1 ("accel/ivpu: Split FW runtime and global memory 
>> buffers")
>> Cc: <stable@vger.kernel.org> # v7.0+
>> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
> 
> Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> 

Applied to drm-misc-fixes.

>> ---
>>   drivers/accel/ivpu/ivpu_fw.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
>> index 107f8ad31050..33c50779c06b 100644
>> --- a/drivers/accel/ivpu/ivpu_fw.c
>> +++ b/drivers/accel/ivpu/ivpu_fw.c
>> @@ -259,6 +259,22 @@ static int ivpu_fw_parse(struct ivpu_device *vdev)
>>           return -EINVAL;
>>       }
>> +    if (!PAGE_ALIGNED(runtime_addr)) {
>> +        ivpu_err(vdev, "Runtime address 0x%llx not page aligned\n", 
>> runtime_addr);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (!PAGE_ALIGNED(runtime_size)) {
>> +        ivpu_err(vdev, "Runtime size %llu not page aligned\n", 
>> runtime_size);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (runtime_size < image_size) {
>> +        ivpu_err(vdev, "Runtime size too small: %llu, image size: 
>> %llu\n",
>> +             runtime_size, image_size);
>> +        return -EINVAL;
>> +    }
>> +
>>       if (!ivpu_is_within_range(image_load_addr, image_size, &vdev- 
>> >hw->ranges.runtime)) {
>>           ivpu_err(vdev, "Invalid firmware load address: 0x%llx and 
>> size %llu\n",
>>                image_load_addr, image_size);
> 
> 

Claude review: accel/ivpu: Add bounds check for firmware runtime memory

[Claude Code Review Bot]

Overall Series Review

Subject: accel/ivpu: Add bounds check for firmware runtime memory
Author: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
Patches: 3
Reviewed: 2026-06-04T16:34:16.789303

---

This is a single patch adding three new validation checks to `ivpu_fw_parse()` for the firmware runtime memory region parsed from the firmware image header. The checks validate page alignment of the runtime address and size, and ensure the runtime region is large enough to hold the firmware image. The intent is correct and the checks are reasonable defensive hardening against malformed firmware images.

However, there is a **placement issue** and a **missing underflow check** that should be addressed. The patch also doesn't quite apply to the current drm-next tree (the context doesn't match), which the submitter should confirm against their target branch.

**Verdict: The patch is a good idea but needs a minor revision.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: accel/ivpu: Add bounds check for firmware runtime memory

[Claude Code Review Bot]

Patch Review

**Positive aspects:**
- The three checks are individually valid and worthwhile. Page alignment of `runtime_addr` and `runtime_size` is important since `ivpu_bo_create_runtime()` ultimately allocates pages, and an unaligned size would waste or mismap memory. The `runtime_size >= image_size` check prevents writing the firmware image beyond the allocated buffer.
- The error messages are clear and include the relevant values for debugging.
- The Fixes tag and stable backport annotation are appropriate.

**Issue 1 (medium): Check placement is too late**

The new checks are inserted *after* the existing `FW_FILE_IMAGE_OFFSET + image_size > fw->file->size` check (around current line 257-260 in drm-next), but *before* the `ivpu_is_within_range(image_load_addr, ...)` check. However, looking at the current tree, the `runtime_addr`/`runtime_size` range check already exists *earlier* at lines 251-255:

```c
if (!ivpu_is_within_range(runtime_addr, runtime_size, &vdev->hw->ranges.runtime)) {
    ivpu_err(vdev, "Invalid firmware runtime address: 0x%llx and size %llu\n",
             runtime_addr, runtime_size);
    return -EINVAL;
}
```

The alignment and sizing checks should be placed **before** this existing `ivpu_is_within_range()` call (i.e., right after lines 245-249 where `runtime_addr`, `runtime_size`, and `image_size` are assigned), not after it. That way, obviously-invalid values (misaligned or undersized) are rejected before being fed into the range check, which provides a more logical validation ordering: basic sanity first, then range containment.

**Issue 2 (medium): Missing underflow check on `runtime_size` computation**

At line 246 of the current tree:
```c
runtime_size = fw_hdr->runtime_size - boot_params_size - fw_version_size;
```

`fw_hdr->runtime_size` is a `u32` (from `vpu_boot_api.h:75`), `boot_params_size` is `SZ_4K` (u64), and `fw_version_size` is `ALIGN(fw_hdr->firmware_version_size, SZ_4K)` (u64). Since `runtime_size` is a local `u64`, if `fw_hdr->runtime_size` is smaller than the sum of the subtractions, this will produce a large wrapping value rather than a negative number. That wrapped value would then pass the `PAGE_ALIGNED` and `runtime_size >= image_size` checks (and even the `ivpu_is_within_range` check could behave unexpectedly with a near-`U64_MAX` size).

Since this patch is already adding validation in this area, it would be worth also adding an underflow guard:

```c
if (fw_hdr->runtime_size < boot_params_size + fw_version_size) {
    ivpu_err(vdev, "Runtime size too small for boot params and fw version\n");
    return -EINVAL;
}
```

This should be placed before the subtraction at line 246.

**Issue 3 (minor): Format specifier correctness**

The `%llu` format specifiers are correct for the `u64` local variables `runtime_addr`, `runtime_size`, and `image_size`. No issue here.

**Issue 4 (minor/nit): The `runtime_size < image_size` check is somewhat redundant**

The later `ivpu_is_within_range(image_load_addr, image_size, &vdev->hw->ranges.runtime)` check combined with `image_load_addr == runtime_addr` (they are both assigned from `fw_hdr->image_load_address`) means the image must fit within the runtime hardware range. However, `runtime_size` as computed here is not the full hardware range — it's the header's `runtime_size` minus boot params and fw version. So the new check `runtime_size < image_size` is a valid additional constraint ensuring the image fits in the *allocated* runtime buffer, not just the hardware range. This is correct and non-redundant.

**Summary:** The patch addresses a real gap in firmware image validation. The recommended changes are: (1) move the new checks earlier, right after lines 245-249, before the existing `ivpu_is_within_range(runtime_addr, ...)` call; (2) add an underflow check on the `runtime_size` subtraction. With those fixes, this should be good to merge.

---
Generated by Claude Code Patch Reviewer