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

[Claude Code Review Bot <claude-review@example.com>]

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