Re: [PATCH] drm/amd/display: avoid NULL hubp deref when filling mpcc_id on DCE

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