Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] This problem wasn't introduced by this patch, but the debugfs trace output formatter uses unbounded `%s` to print firmware-provided assertion strings, which can read past the end of the allocation if the strings lack a NUL terminator. -- commit 97d5534fd4f9139da050fa97ad4093d575e6bdd3 Author: Alessio Belle <[email protected]> drm/imagination: Avoid initialisation of unused FW trace buffer pointer This commit removes the initialization of tracebuf_space->trace_buffer, an unused pointer in the firmware interface structure. The trace buffer source pointer continues to be used as before. > diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c > b/drivers/gpu/drm/imagination/pvr_fw_trace.c > index 6bb5baa6c41b7..813b7b0c8d4e9 100644 > --- a/drivers/gpu/drm/imagination/pvr_fw_trace.c > +++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c > @@ -101,7 +101,6 @@ tracebuf_ctrl_init(void *cpu_ptr, void *priv) > pvr_fw_object_get_fw_addr(trace_buffer->buf_obj, > > &tracebuf_space->trace_buffer_fw_addr); > > - tracebuf_space->trace_buffer = trace_buffer->buf; > tracebuf_space->trace_pointer = 0; > } > } [Severity: High] This isn't a bug introduced by this patch, but looking at another part of pvr_fw_trace.c in fw_trace_seq_show(), the debugfs trace output formatter uses unbounded %s format specifiers to print firmware-provided assertion strings: seq_printf(s, "ASSERTION %s failed at %s:%u", trace_seq_data->assert_buf.info, trace_seq_data->assert_buf.path, trace_seq_data->assert_buf.line_num); Can this read past the end of the allocation if the firmware provides strings without a NUL terminator? Since assert_buf is the last member of trace_seq_data, reading past the end of info or path could read out of the structure's bounds into adjacent heap memory. If the unbounded string read crosses a page boundary into an unmapped page, it could cause a kernel panic. Additionally, it might leak adjacent kernel heap memory into the debugfs output. Would it be safer to use %.*s with the maximum buffer size, or forcefully NUL-terminate the buffers after copying them from the firmware? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260910-b4-avoid-init-of-unused-fw-trace-buffer-pointer-v1-0-7e1027438...@imgtec.com?part=2
