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

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

[Honglei Huang]

HUBP only exists on DCN. On ASICs that use DCE, such as Vega20, the display
pipe never owns a HUBP, so pipe_ctx->plane_res.hubp stays NULL.

The block sequence rework now fills mpcc_id inside the generic helpers in
dc_hw_sequencer.c by reading pipe_ctx->plane_res.hubp->inst. On a DCE ASIC
that read follows the NULL pointer and faults at offset 0x88, where the
inst field sits in struct hubp, and the box dies on the first modeset:

  BUG: kernel NULL pointer dereference, address: 0000000000000088
  #PF: supervisor read access in kernel mode
  Oops: 0000 [#1] SMP NOPTI
  CPU: 14 UID: 0 PID: 531 Comm: plymouthd Not tainted 7.1.0 #70
  RIP: 0010:hwss_set_output_transfer_func+0x69/0xc0 [amdgpu]
  Call Trace:
   dce110_program_front_end_for_pipe+0x4d7/0x5c0 [amdgpu]
   dce110_apply_ctx_for_surface+0xf1/0x270 [amdgpu]
   commit_planes_for_stream+0xba1/0x1c80 [amdgpu]
   update_planes_and_stream_v2+0x297/0x6f0 [amdgpu]
   dc_update_state_prepare+0x6f/0x1c0 [amdgpu]
   dc_update_state+0x41/0x60 [amdgpu]
   dc_update_planes_and_stream+0x43/0x70 [amdgpu]
   amdgpu_dm_atomic_commit_tail+0x1a6f/0x4120 [amdgpu]
   commit_tail+0xc5/0x1a0
   drm_atomic_helper_commit+0x137/0x160
   drm_atomic_commit+0xaf/0xf0
   drm_client_modeset_commit_atomic+0x1ec/0x230
   drm_client_modeset_commit_locked+0x5b/0x170
   drm_client_modeset_commit+0x27/0x50
   __drm_fb_helper_restore_fbdev_mode_unlocked+0xe8/0x110
   drm_fbdev_client_restore+0x12/0x20
   drm_client_dev_restore+0xbb/0x100

DCE never uses mpcc_id, so 0 is a fine value when there is no HUBP. Add a
small helper that returns the HUBP instance, or 0 when it is missing, and
use it for every mpcc_id assignment. DCN is unchanged since HUBP is always
there.

Fixes: f879f53407f0 ("drm/amd/display: Refactor DPP_SET_OUTPUT_TRANSFER_FUNC to drop pipe_ctx")
Fixes: 9403e15b2ab1 ("drm/amd/display: Refactor DPP_PROGRAM_GAMUT_REMAP to drop pipe_ctx param")
Cc: Tomasz Siemek <tomasz.siemek@amd.com>
Cc: Wayne Lin <wayne.lin@amd.com>
Cc: Alvin Lee <alvin.lee2@amd.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
 .../drm/amd/display/dc/core/dc_hw_sequencer.c | 26 +++++++++++++------
 1 file changed, 18 insertions(+), 8 deletions(-)

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
index 11411fa946..7676aa4485 100644
--- 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)
+{
+	return pipe_ctx->plane_res.hubp ? pipe_ctx->plane_res.hubp->inst : 0;
+}
+
 void hwss_build_fast_sequence(struct dc *dc,
 		struct dc_dmub_cmd *dc_dmub_cmd,
 		unsigned int dmub_cmd_count,
@@ -1073,7 +1083,7 @@ void hwss_build_fast_sequence(struct dc *dc,
 					params->dpp = current_mpc_pipe->plane_res.dpp;
 					params->mpc = dc->res_pool->mpc;
 					params->xfm = current_mpc_pipe->plane_res.xfm;
-					params->mpcc_id = current_mpc_pipe->plane_res.hubp->inst;
+					params->mpcc_id = hwss_pipe_mpcc_id(current_mpc_pipe);
 					params->plane = current_mpc_pipe->plane_state;
 					params->stream = current_mpc_pipe->stream;
 					params->is_top_pipe = current_mpc_pipe->top_pipe == NULL;
@@ -1118,7 +1128,7 @@ void hwss_build_fast_sequence(struct dc *dc,
 				otf_params->dpp = current_mpc_pipe->plane_res.dpp;
 				otf_params->xfm = current_mpc_pipe->plane_res.xfm;
 				otf_params->mpc = dc->res_pool->mpc;
-				otf_params->mpcc_id = current_mpc_pipe->plane_res.hubp->inst;
+				otf_params->mpcc_id = hwss_pipe_mpcc_id(current_mpc_pipe);
 				otf_params->is_top_pipe = resource_is_pipe_type(current_mpc_pipe, OPP_HEAD);
 				otf_params->stream = current_mpc_pipe->stream;
 				block_sequence[*num_steps].func = DPP_SET_OUTPUT_TRANSFER_FUNC;
@@ -1128,13 +1138,13 @@ void hwss_build_fast_sequence(struct dc *dc,
 				dc->hwss.update_visual_confirm_color) {
 				block_sequence[*num_steps].params.update_visual_confirm_params.dc = dc;
 				block_sequence[*num_steps].params.update_visual_confirm_params.pipe_ctx = current_mpc_pipe;
-				block_sequence[*num_steps].params.update_visual_confirm_params.mpcc_id = current_mpc_pipe->plane_res.hubp->inst;
+				block_sequence[*num_steps].params.update_visual_confirm_params.mpcc_id = hwss_pipe_mpcc_id(current_mpc_pipe);
 				block_sequence[*num_steps].func = MPC_UPDATE_VISUAL_CONFIRM;
 				(*num_steps)++;
 			}
 			if (current_mpc_pipe->stream->update_flags.bits.out_csc) {
 				block_sequence[*num_steps].params.power_on_mpc_mem_pwr_params.mpc = dc->res_pool->mpc;
-				block_sequence[*num_steps].params.power_on_mpc_mem_pwr_params.mpcc_id = current_mpc_pipe->plane_res.hubp->inst;
+				block_sequence[*num_steps].params.power_on_mpc_mem_pwr_params.mpcc_id = hwss_pipe_mpcc_id(current_mpc_pipe);
 				block_sequence[*num_steps].params.power_on_mpc_mem_pwr_params.power_on = true;
 				block_sequence[*num_steps].func = MPC_POWER_ON_MPC_MEM_PWR;
 				(*num_steps)++;
@@ -1837,7 +1847,7 @@ void hwss_add_dpp_program_gamut_remap(struct block_sequence_state *seq_state,
 		params->xfm = pipe_ctx->plane_res.xfm;
 		params->dpp = pipe_ctx->plane_res.dpp;
 		params->mpc = pipe_ctx->stream->ctx->dc->res_pool->mpc;
-		params->mpcc_id = pipe_ctx->plane_res.hubp->inst;
+		params->mpcc_id = hwss_pipe_mpcc_id(pipe_ctx);
 		params->plane = pipe_ctx->plane_state;
 		params->stream = pipe_ctx->stream;
 		params->is_top_pipe = pipe_ctx->top_pipe == NULL;
@@ -1883,7 +1893,7 @@ void hwss_add_dpp_set_output_transfer_func(struct block_sequence_state *seq_stat
 			.xfm = pipe_ctx->plane_res.xfm,
 			.dpp = pipe_ctx->plane_res.dpp,
 			.mpc = dc->res_pool->mpc,
-			.mpcc_id = pipe_ctx->plane_res.hubp->inst,
+			.mpcc_id = hwss_pipe_mpcc_id(pipe_ctx),
 			.is_top_pipe = resource_is_pipe_type(pipe_ctx, OPP_HEAD),
 			.stream = pipe_ctx->stream,
 		};
@@ -1900,7 +1910,7 @@ void hwss_set_output_transfer_func(struct dc *dc, struct pipe_ctx *pipe_ctx)
 				.xfm = pipe_ctx->plane_res.xfm,
 				.dpp = pipe_ctx->plane_res.dpp,
 				.mpc = dc->res_pool->mpc,
-				.mpcc_id = pipe_ctx->plane_res.hubp->inst,
+				.mpcc_id = hwss_pipe_mpcc_id(pipe_ctx),
 				.is_top_pipe = resource_is_pipe_type(pipe_ctx, OPP_HEAD),
 				.stream = pipe_ctx->stream,
 			}
@@ -3840,7 +3850,7 @@ void hwss_program_gamut_remap(struct pipe_ctx *pipe_ctx)
 			.xfm = pipe_ctx->plane_res.xfm,
 			.dpp = pipe_ctx->plane_res.dpp,
 			.mpc = dc->res_pool->mpc,
-			.mpcc_id = pipe_ctx->plane_res.hubp->inst,
+			.mpcc_id = hwss_pipe_mpcc_id(pipe_ctx),
 			.stream = pipe_ctx->stream,
 			.plane = pipe_ctx->plane_state,
 			.is_top_pipe = pipe_ctx->top_pipe == NULL,
-- 
2.34.1

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

[sashiko-bot]

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 <honghuan@amd.com>

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/20260805012350.1458583-1-honghuan@amd.com?part=1