Re: [PATCH] drm/i915/hdmi: Fix potential overflow while using intel_encoder_to_tc

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 03 Sep 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> The helper intel_encoder_to_tc() can potentially return TC_PORT_NONE
> (-1) and cause overflow while computing ddc pins in
> icl_encoder_to_ddc_pin().
>
> Check for TC_PORT_NONE before deriving the ddc pins for TC port.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index cbee628eb26b..85f70cedc40c 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2791,10 +2791,16 @@ static u8 icl_encoder_to_ddc_pin(struct intel_encoder *encoder)
>  	struct intel_display *display = to_intel_display(encoder);
>  	enum port port = encoder->port;
>  
> -	if (intel_encoder_is_combo(encoder))
> +	if (intel_encoder_is_combo(encoder)) {
>  		return GMBUS_PIN_1_BXT + port;
> -	else if (intel_encoder_is_tc(encoder))
> -		return GMBUS_PIN_9_TC1_ICP + intel_encoder_to_tc(encoder);
> +	} else if (intel_encoder_is_tc(encoder)) {
> +		enum tc_port tc_port = intel_encoder_to_tc(encoder);

intel_encoder_to_tc() can only return TC_PORT_NONE if intel_phy_is_tc()
== false. But intel_encoder_is_tc() just above means intel_phy_is_tc()
== true.

This case can't happen, it's a static analyzer being overzealous.

Adding checks like this to please a static analyzer leads to misery,
because it adds unnecessary code to maintain, and it will never be run.

I think it would be more interesting to make intel_port_to_tc() WARN on
!intel_phy_is_tc(), return a bogus >= 0 port, and ensure all callers
only call it on TC ports.

This obviously leads to issues if it happens, but hey, it shouldn't
happen, and intel_encoder_to_tc() returning TC_PORT_NONE is *already*
such a case. Just move it to lower levels.

If we start checking for every impossible situation, and propagating
errors for them, our codebase will be 90% error handling.


BR,
Jani.


> +
> +		if (tc_port != TC_PORT_NONE)
> +			return GMBUS_PIN_9_TC1_ICP + tc_port;
> +
> +		drm_WARN(display->drm, 1, "Invalid TC port\n");
> +	}
>  
>  	drm_WARN(display->drm, 1, "Unknown port:%c\n", port_name(port));
>  	return GMBUS_PIN_2_BXT;

-- 
Jani Nikula, Intel