Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The patch resolves a NULL pointer dereference of `hubp` on DCE ASICs in `dc_hw_sequencer.c` by using a helper to fetch `mpcc_id`, but misses identical pre-existing vulnerabilities in `dc.c` and `dc_stream.c`. When specific `visual_confirm` or `validate_dml_output` debug options are enabled (which are documented options passed via module parameters or debugfs), the driver unconditionally dereferences `pipe_ctx->plane_res.hubp` and calls uninitialized function pointers (like `dc->hwss.update_visual_confirm_color`, which is NULL on DCE). This leads to an immediate kernel panic on DCE hardware. -- --- Patch [1]: [PATCH] drm/amd/display: avoid NULL hubp deref when filling mpcc_id on DCE --- commit 4049d13de015ecc06980243ec06ca6c8b79266f3 Author: Honglei Huang <[email protected]> drm/amd/display: avoid NULL hubp deref when filling mpcc_id on DCE The commit fixes a NULL pointer dereference on DCE ASICs by introducing a helper to safely fetch the mpcc_id when a HUBP instance is missing. > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c > b/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c > --- a/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c > @@ -729,6 +729,16 @@ void get_fams2_visual_confirm_color( > } > } > > +/* > + * Return the MPCC instance for a pipe. On DCN it matches the HUBP instance. > + * DCE has no HUBP and ignores mpcc_id, so return 0 there instead of chasing > a > + * NULL pointer. > + */ > +static int hwss_pipe_mpcc_id(const struct pipe_ctx *pipe_ctx) > +{ [Severity: High] This is a pre-existing issue, but while this patch resolves the NULL dereferences of plane_res.hubp in dc_hw_sequencer.c, are there still identical vulnerabilities in dc.c and dc_stream.c on DCE ASICs? For example, in drivers/gpu/drm/amd/display/dc/core/dc.c in dc_commit_state_no_check(), if VISUAL_CONFIRM_SUBVP or VISUAL_CONFIRM_MCLK_SWITCH are enabled, the code unconditionally dereferences hubp->inst: dc->hwss.update_visual_confirm_color(dc, pipe_ctx, pipe_ctx->plane_res.hubp->inst); Similarly, in drivers/gpu/drm/amd/display/dc/core/dc_stream.c in dc_stream_program_cursor_position(), if VISUAL_CONFIRM_HW_CURSOR is enabled: dc->hwss.update_visual_confirm_color(dc, pipe_ctx, pipe_ctx->plane_res.hubp->mpcc_id); Additionally, dc_commit_state_no_check() unconditionally accesses hubp->funcs->validate_dml_output on DCE when validate_dml_output is enabled. Does the driver also call a NULL function pointer on DCE in these paths since dc->hwss.update_visual_confirm_color might not be initialized for DCE? Could the new hwss_pipe_mpcc_id() helper be exported and used to fix these remaining paths, and guards added for the function pointer calls? > + return pipe_ctx->plane_res.hubp ? pipe_ctx->plane_res.hubp->inst : 0; > +} > + > void hwss_build_fast_sequence(struct dc *dc, [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
