> Subject: [PATCH] drm/i915/cmtg: Fix array-index-out-of-bounds in
> intel_cmtg_disable
>
> to_cmtg_transcoder() returns INVALID_TRANSCODER (-1) for any
> cpu_transcoder other than TRANSCODER_A/B. intel_cmtg_disable() then uses
> the resulting cmtg_transcoder to index the per-transcoder register array via
> TRANS_VRR_CTL(), but unlike the other CMTG helpers it does not check for
> INVALID_TRANSCODER first. The only guard is crtc->cmtg.enabled, which is a
> sticky flag on struct intel_crtc and can be set while the crtc_state being
> disabled has a cpu_transcoder that does not map to a CMTG transcoder (e.g.
> during a big-joiner reconfiguration that moves the eDP across pipes).
>
> This results in a negative array index (trans_offsets[-1]) and a UBSAN
> splat:
>
> UBSAN: array-index-out-of-bounds in .../display/intel_cmtg.c:187:24
> intel_cmtg_disable+0x395/0x3d0 [xe]
> intel_old_crtc_state_disables+0xfb/0x1f0 [xe]
> intel_atomic_commit_tail+0xca6/0x2040 [xe]
>
> Bail out early when cmtg_transcoder is INVALID_TRANSCODER, matching the
> guard already used by the other CMTG helpers.
>
> Fixes: 3bb44e8d421a ("drm/i915/cmtg: Modify existing hook to disable
> CMTG")
> Signed-off-by: Animesh Manna <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_cmtg.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> b/drivers/gpu/drm/i915/display/intel_cmtg.c
> index 6da28c185080..dcd29ebdb432 100644
> --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> @@ -183,6 +183,9 @@ void intel_cmtg_disable(const struct intel_crtc_state
> *crtc_state)
> if (!crtc->cmtg.enabled)
> return;
>
> + if (cmtg_transcoder == INVALID_TRANSCODER)
Hmm weird that we come this far with an invalid transcoder, considering that
cmtg.enabled passed.
I think we need to make this a drm_WARN_ON(cmtg_transcoder ==
INVALID_TRANSCODER) and return early
Also to get this splat out of the way we can either inside this function or
where intel_cmtg_disable is called can be guarded by
intel_cmtg_is_allowed()
Like this:
if (crtc->cmtg.enabled && intel_cmtg_is_allowed(old_crtc_state)) {
intel_cmtg_set_clk_select(old_crtc_state);
intel_cmtg_disable(old_crtc_state);
}
if (crtc->cmtg.enabled && (intel_crtc_vrr_enabling(state, crtc)) &&
&& intel_cmtg_is_allowed(new_crtc_state))) {
intel_cmtg_set_clk_select(new_crtc_state);
intel_cmtg_disable(new_crtc_state);
}
I think this is a more full proof way of fixing this issue
Regards,
Suraj Kandpal
> + return;
> +
> crtc->cmtg.enabled = false;
> intel_de_rmw(display, TRANS_VRR_CTL(display, cmtg_transcoder),
> VRR_CTL_VRR_ENABLE | VRR_CTL_FLIP_LINE_EN, 0);
> --
> 2.29.0
