From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: accel/ivpu: Add buffer overflow check in MS get_info_ioctl Date: Thu, 04 Jun 2026 16:35:51 +1000 Message-ID: In-Reply-To: <20260529120841.135852-1-andrzej.kacprowski@linux.intel.com> References: <20260529120841.135852-1-andrzej.kacprowski@linux.intel.com> <20260529120841.135852-1-andrzej.kacprowski@linux.intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Purpose and correctness:** The call chain is: 1. `ivpu_jsm_metric_streamer_info()` is called with `ivpu_bo_size(bo)` as t= he buffer size limit (line 290), and fills `info_size` (a `u64`) with the a= ctual data size returned by firmware. 2. `info_size` is then used directly at line 299 in `copy_to_user(..., info= _size)`. If firmware returns an `info_size` larger than `ivpu_bo_size(bo)`, the `cop= y_to_user` would read past the end of the BO's mapped memory. The new check= correctly catches this case before the data is used. The placement is correct =E2=80=94 right after the JSM call succeeds (line = 291-292) and before either the `args->buffer_size` comparison or the `copy_= to_user`. This ensures the firmware-returned size is validated against the = kernel buffer before any further use. **Format string nit:** ```c ivpu_warn_ratelimited(vdev, "MS info overflow: %#llx > %#zx\n", info_size, ivpu_bo_size(bo)); ``` `info_size` is `u64` =E2=80=94 in the kernel, the canonical format specifie= r for `u64` is `%llu` (or `%#llx` for hex, which is used here). That's fine= . `ivpu_bo_size()` returns `size_t`, and `%zx` is appropriate for that. So = the format specifiers are correct. However, one could argue that a decimal format would be more readable for s= izes in a warning message (comparing `0x10001` vs `0x10000` is harder to ey= eball than `65537 > 65536`). This is purely a style preference and not a bu= g. **Error code:** `-EOVERFLOW` is a reasonable choice for this condition =E2=80=94 it clearly= conveys that the returned data would overflow the allocated buffer. Some d= rivers use `-ENOSPC` for related conditions, but `EOVERFLOW` better capture= s the "firmware returned more than it should have" semantics, distinguishin= g it from the existing `-ENOSPC` check on the user buffer (line 294). **No concerns with the fix itself.** The patch is correct, minimal, and add= resses a real vulnerability where firmware could cause an out-of-bounds ker= nel memory read that gets copied to userspace. Reviewed-by assessment: **Acceptable as-is.** --- Generated by Claude Code Patch Reviewer