From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/dp/mst: fix OOB reads in remote DPCD/I2C sideband reply parsers Date: Sat, 16 May 2026 16:07:32 +1000 Message-ID: In-Reply-To: <20260510201733.2882224-1-ashutoshdesai993@gmail.com> References: <20260510201733.2882224-1-ashutoshdesai993@gmail.com> <20260510201733.2882224-1-ashutoshdesai993@gmail.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 **DPCD read fix (line 883):** ```c - if (idx > raw->curlen) + if (idx + repmsg->u.remote_dpcd_read_ack.num_bytes > raw->curlen) ``` Correct. The old check only validated that `idx` (the position after readin= g `num_bytes` from the header) was within bounds. It did not account for th= e subsequent `memcpy` of `num_bytes` bytes starting at `idx`. Since `idx` i= s `int` (value ~3 at this point) and `num_bytes` is `u8` (max 255), the add= ition cannot overflow and the comparison against `u8 curlen` is safe. **I2C read fix (lines 919-920):** ```c - /* TODO check */ + if (idx + repmsg->u.remote_i2c_read_ack.num_bytes > raw->curlen) + goto fail_len; ``` Correct. This resolves a longstanding TODO =E2=80=94 the I2C read path had = **no** bounds check at all before the `memcpy`. The fix mirrors the DPCD re= ad fix and uses the same `fail_len` label pattern already present in the fu= nction. **Boundary condition correctness:** The check `idx + num_bytes > curlen` is= the right comparison. Valid message data occupies indices `0` through `cur= len - 1`, so a copy of `num_bytes` starting at `idx` requires `idx + num_by= tes <=3D curlen`. The `>` test correctly rejects violations. **Minor observation:** The commit message's v2 changelog says "Drop separat= e `idx > curlen` check" =E2=80=94 this accurately describes the DPCD read c= ase where the old check was replaced (since `idx + num_bytes > curlen` with= `u8 num_bytes >=3D 0` subsumes `idx > curlen`). For the I2C case there was= nothing to drop (only the TODO comment), but the changelog is adequate. **No issues found.** The patch is correct, minimal, and appropriate for `Cc= : stable`. --- Generated by Claude Code Patch Reviewer