> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Luca 
> Coelho
> Sent: Tuesday, 20 May 2025 11.27
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 1/2] drm/i915/dkl: return if tc_port is invalid in 
> dkl_phy_set_hip_idx()
> 
> In dkl_phy_set_hip_idx(), we may try to shift a value negatively, whose 
> behavior is undefined.  This can happen because we define
> TC_PORT_NONE to -1, so theoretically tc_port could be -1.  We will then use 
> tc_port to shift to the correct address of the specified
> port, but if it's negative, anything can happen.
> 
> If this happens or tc_port exceeds I915_MAX_TC_PORTS, it's safer to return 
> with a warning than risk an invalid write.
> 

To me the approach looks valid.

Reviewed-by: Mika Kahola <mika.kah...@intel.com>

> Signed-off-by: Luca Coelho <luciano.coe...@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dkl_phy.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.c 
> b/drivers/gpu/drm/i915/display/intel_dkl_phy.c
> index dad7192132ad..35e919eae369 100644
> --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.c
> @@ -25,7 +25,9 @@ dkl_phy_set_hip_idx(struct intel_display *display, struct 
> intel_dkl_phy_reg reg)  {
>       enum tc_port tc_port = DKL_REG_TC_PORT(reg);
> 
> -     drm_WARN_ON(display->drm, tc_port < TC_PORT_1 || tc_port >= 
> I915_MAX_TC_PORTS);
> +     if (drm_WARN_ON(display->drm,
> +                     tc_port < TC_PORT_1 || tc_port >= I915_MAX_TC_PORTS))
> +             return;
> 
>       intel_de_write(display,
>                      HIP_INDEX_REG(tc_port),
> --
> 2.47.2

Reply via email to