Re: [PATCH v2] drm/i915/dp: Use array size for intersect_rates() bound

"Borah, Chaitanya Kumar" <[email protected]>
Newsgroups org.freedesktop.lists.intel-gfx,org.freedesktop.lists.intel-xe
Message-ID <[email protected]>

On 8/4/2026 4:25 PM, Suraj Kandpal wrote:
> Take an explicit common_len parameter and pass
> ARRAY_SIZE(intel_dp->common_rates) at the call site, so the bound is
> tied to the destination array.
> 
> Fixes: e6bda3e4cb43 ("drm/i915: Avoid overflowing the DP link rate arrays")
> Signed-off-by: Suraj Kandpal <[email protected]>
> ---
> v1 -> v2:
> -Use latest baseline (Jani)
> 
>   drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7f13595f40c1..1f1324c99130 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -625,13 +625,13 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
>   
>   static int intersect_rates(const int *source_rates, int source_len,
>   			   const int *sink_rates, int sink_len,
> -			   int *common_rates)
> +			   int *common_rates, int common_len)
>   {
>   	int i = 0, j = 0, k = 0;
>   
>   	while (i < source_len && j < sink_len) {
>   		if (source_rates[i] == sink_rates[j]) {
> -			if (WARN_ON(k >= DP_MAX_SUPPORTED_RATES))
> +			if (WARN_ON(k >= common_len))

DP_MAX_SUPPORTED_RATES made more sense here because it keeps the size of 
the common rate (and sink rate) from drifting away from the standard macro.

>   				return k;
>   			common_rates[k] = source_rates[i];
>   			++k;
> @@ -660,6 +660,7 @@ int intel_dp_rate_index(const int *rates, int len, int rate)
>   
>   static void intel_dp_get_common_rates(struct intel_dp *intel_dp,
>   				      int common_rates[DP_MAX_SUPPORTED_RATES],
> +				      int common_len,
>   				      int *num_common_rates)
>   {
>   	struct intel_display *display = to_intel_display(intel_dp);
> @@ -671,7 +672,8 @@ static void intel_dp_get_common_rates(struct intel_dp *intel_dp,
>   					    intel_dp->num_source_rates,
>   					    intel_dp->sink_rates,
>   					    intel_dp->num_sink_rates,
> -					    common_rates);
> +					    common_rates,
> +					    common_len);
>   
>   	/* Paranoia, there should always be something in common. */
>   	if (drm_WARN_ON(display->drm, *num_common_rates == 0)) {
> @@ -687,7 +689,8 @@ static bool intel_dp_set_common_link_params(struct intel_dp *intel_dp)
>   	int common_rates[DP_MAX_SUPPORTED_RATES];
>   	bool params_changed = false;
>   
> -	intel_dp_get_common_rates(intel_dp, common_rates, &num_common_rates);
> +	intel_dp_get_common_rates(intel_dp, common_rates, ARRAY_SIZE(common_rates),
> +				  &num_common_rates);

Is this patch in response to an actual bug or just a defensive code 
change? If it is the later, does it add much value?

common_rates and sink_rate are already locked in with 
DP_MAX_SUPPORTED_RATES. There is a hazard of them drifting away from a 
common size for which we could add a static enforcement of

BUILD_BUG_ON(ARRAY_SIZE(common_rates) != ARRAY_SIZE(sink_rates));

Something similar is done in intel_dp_set_dpcd_sink_rates() to keep the 
sink_rate within limits.

Regardless, if there is no actual bug then the Fixes tag is unjustified.

==

Chaitanya



>   	if (intel_dp_link_caps_update(intel_dp->link.caps,
>   				      common_rates, num_common_rates,
>   				      intel_dp_get_max_common_lane_count(intel_dp),
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.