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/imagination: Fix segfault when updating ftrace mask Date: Tue, 28 Apr 2026 15:07:03 +1000 Message-ID: In-Reply-To: <20260427-ftrace_fix-v3-1-e081530759a8@imgtec.com> References: <20260427-ftrace_fix-v3-0-e081530759a8@imgtec.com> <20260427-ftrace_fix-v3-1-e081530759a8@imgtec.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 **Verdict: Good to merge.** The fix is correct and minimal. Looking at the current source (`pvr_fw_trac= e.c`), the debugfs get/set functions both expect `struct pvr_device *`: ```c static int pvr_fw_trace_mask_get(void *data, u64 *value) { struct pvr_device *pvr_dev =3D data; *value =3D pvr_dev->fw_dev.fw_trace.group_mask; return 0; } static int pvr_fw_trace_mask_set(void *data, u64 value) { struct pvr_device *pvr_dev =3D data; ... } ``` But the buggy code passed `fw_trace` (a `struct pvr_fw_trace *`): ```c - debugfs_create_file("trace_mask", 0600, dir, fw_trace, + debugfs_create_file("trace_mask", 0600, dir, pvr_dev, &pvr_fw_trace_mask_fops); ``` When `pvr_fw_trace_mask_set` then dereferences `pvr_dev->fw_dev.fw_trace.gr= oup_mask` using the mistyped pointer, it reads/writes garbage memory, causi= ng the NULL pointer dereference shown in the crash log. The fix correctly p= asses `pvr_dev` which is already available as the function parameter. The `Fixes:` tag (`a331631496a0 "drm/imagination: Simplify module parameter= s"`) is appropriate =E2=80=94 that commit presumably changed the private da= ta from `pvr_dev` to `fw_trace` as part of a refactor, introducing the regr= ession. The crash trace in the commit message is helpful for backporting and triage= . No concerns. --- Generated by Claude Code Patch Reviewer