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/exynos: vidi: fix to avoid directly dereferencing user pointer Date: Mon, 23 Feb 2026 06:11:03 +1000 Message-ID: In-Reply-To: <20260220033550.124346-3-aha310510@gmail.com> References: <20260220033550.124346-1-aha310510@gmail.com> <20260220033550.124346-3-aha310510@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This patch fixes a security issue where the user-space EDID pointer from the ioctl struct was directly cast to a kernel pointer and dereferenced. The fix properly uses `copy_from_user()` to first copy the EDID header (to read the `extensions` count), then allocates a kernel buffer and copies the full EDID data. > + const void __user *edid_userptr = u64_to_user_ptr(vidi->edid); > + void *edid_buf; > + struct edid hdr; > size_t size; > > - raw_edid = (const struct edid *)(unsigned long)vidi->edid; > - size = (raw_edid->extensions + 1) * EDID_LENGTH; > + if (copy_from_user(&hdr, edid_userptr, sizeof(hdr))) > + return -EFAULT; > + > + size = (hdr.extensions + 1) * EDID_LENGTH; The size computation is safe from overflow: `hdr.extensions` is a `u8` (max 255), so the maximum size is `256 * 128 = 32768` bytes. The use of `u64_to_user_ptr()` is the correct way to convert the `__u64` ioctl field to a `__user` pointer. > + edid_buf = kmalloc(size, GFP_KERNEL); > + if (!edid_buf) > + return -ENOMEM; > + > + if (copy_from_user(edid_buf, edid_userptr, size)) { > + kfree(edid_buf); > + return -EFAULT; > + } > + > + drm_edid = drm_edid_alloc(edid_buf, size); > + kfree(edid_buf); `drm_edid_alloc()` makes its own internal copy of the EDID data, so freeing `edid_buf` immediately after is correct. The error paths properly free `edid_buf` before returning. There is a theoretical TOCTOU between the header copy and the full copy (user space could modify the `extensions` field between the two `copy_from_user` calls), but this is harmless: the size is computed from the kernel copy of the header, so the kernel buffer is always correctly sized. If the user modifies the data between copies, the subsequent `drm_edid_valid()` check (present in the existing code below this diff) will catch any inconsistency. No issues found. --- Generated by Claude Code Patch Reviewer