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: Fix signed integer truncation in IPC receive Date: Thu, 04 Jun 2026 13:53:03 +1000 Message-ID: In-Reply-To: <20260601161643.229342-1-andrzej.kacprowski@linux.intel.com> References: <20260601161643.229342-1-andrzej.kacprowski@linux.intel.com> <20260601161643.229342-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 **The bug is real and clearly explained.** Looking at the existing code: ```c u32 size =3D min_t(int, rx_msg->ipc_hdr->data_size, sizeof(*jsm_msg)); ``` `rx_msg->ipc_hdr->data_size` is `u32` (confirmed in `ivpu_ipc.h:38`), and `= sizeof(*jsm_msg)` is `size_t`. The `min_t(int, ...)` cast truncates both to= signed `int`. If firmware-supplied `data_size` is >=3D 0x80000000, the cas= t produces a negative value, which "wins" the min comparison, then gets ass= igned back to `u32 size` as a huge value, and the subsequent `memcpy(jsm_ms= g, rx_msg->jsm_msg, size)` at line 291 overflows. **The fix is correct:** ```c u32 size =3D min(rx_msg->ipc_hdr->data_size, sizeof(*jsm_msg)); ``` - `data_size` is `u32`, `sizeof(...)` is `size_t` (unsigned). The kernel's = `min()` macro handles type promotion correctly for unsigned types =E2=80=94= `u32` will promote to `size_t` in the comparison, which is safe. - The result is capped to `sizeof(*jsm_msg)` which fits in `u32`, so the as= signment is also safe. **Minor observations (not blocking):** - The `sizeof(*jsm_msg)` operand has type `size_t`, while `data_size` has t= ype `u32`. On 64-bit kernels these differ in width. The kernel's `min()` ma= cro (using `__types_ok()`) permits this comparison when both are unsigned a= nd one promotes safely. This works correctly, but if the build bot flags a = type mismatch warning (some older kernel versions were stricter), `min_t(u3= 2, ...)` would be the fallback. This should be fine on current kernels. - The commit message and changelog are clean. The `Fixes:` tag references t= he correct commit that introduced `min_t(int, ...)`, and the `Cc: stable` a= nnotation with `# v6.12+` is appropriate. **Reviewed-by recommendation: This patch is correct and suitable for mergin= g.** --- Generated by Claude Code Patch Reviewer