[PATCH v2] drm/i915/edp: Check supported link rates DPCD read

[PATCH v2] drm/i915/edp: Check supported link rates DPCD read

[Nikita Zhandarovich]

intel_edp_set_sink_rates() reads DP_SUPPORTED_LINK_RATES into a local
stack array and then parses the array unconditionally. If the read
fails, the array contents are not valid and may result in bogus sink
link rates being used.

Use drm_dp_dpcd_read_data() and clear the sink rate array on failure,
so the existing parser falls back to the default sink rate handling.

Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.

Fixes: 68f357cb7347 ("drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4")
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
---
v1 -> v2:
- Use drm_dp_dpcd_read_data() instead of drm_dp_dpcd_read().
- Avoid the goto by clearing sink_rates on read failure, as suggested by
  Jani Nikula.
- Adjust patch description.

 drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 6ef2a0043cda..5c3e816b0135 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4678,10 +4678,17 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
 
 	if (intel_dp->edp_dpcd[0] >= DP_EDP_14) {
 		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
+		int ret;
 		int i;
 
-		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
-				 sink_rates, sizeof(sink_rates));
+		ret = drm_dp_dpcd_read_data(&intel_dp->aux,
+					    DP_SUPPORTED_LINK_RATES,
+					    sink_rates, sizeof(sink_rates));
+		if (ret < 0) {
+			drm_dbg_kms(display->drm,
+				    "Unable to read eDP supported link rates, using default rates\n");
+			memset(sink_rates, 0, sizeof(sink_rates));
+		}
 
 		for (i = 0; i < ARRAY_SIZE(sink_rates); i++) {
 			int rate;

Re: [PATCH v2] drm/i915/edp: Check supported link rates DPCD read

[Jani Nikula]

On Fri, 29 May 2026, Nikita Zhandarovich <n.zhandarovich@fintech.ru> wrote:
> intel_edp_set_sink_rates() reads DP_SUPPORTED_LINK_RATES into a local
> stack array and then parses the array unconditionally. If the read
> fails, the array contents are not valid and may result in bogus sink
> link rates being used.
>
> Use drm_dp_dpcd_read_data() and clear the sink rate array on failure,
> so the existing parser falls back to the default sink rate handling.
>
> Found by Linux Verification Center (linuxtesting.org) with static
> analysis tool SVACE.
>
> Fixes: 68f357cb7347 ("drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
> v1 -> v2:
> - Use drm_dp_dpcd_read_data() instead of drm_dp_dpcd_read().
> - Avoid the goto by clearing sink_rates on read failure, as suggested by
>   Jani Nikula.
> - Adjust patch description.
>
>  drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6ef2a0043cda..5c3e816b0135 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4678,10 +4678,17 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>  
>  	if (intel_dp->edp_dpcd[0] >= DP_EDP_14) {
>  		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
> +		int ret;
>  		int i;
>  
> -		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
> -				 sink_rates, sizeof(sink_rates));
> +		ret = drm_dp_dpcd_read_data(&intel_dp->aux,
> +					    DP_SUPPORTED_LINK_RATES,
> +					    sink_rates, sizeof(sink_rates));
> +		if (ret < 0) {
> +			drm_dbg_kms(display->drm,
> +				    "Unable to read eDP supported link rates, using default rates\n");
> +			memset(sink_rates, 0, sizeof(sink_rates));
> +		}
>  
>  		for (i = 0; i < ARRAY_SIZE(sink_rates); i++) {
>  			int rate;

-- 
Jani Nikula, Intel

Re: [PATCH v2] drm/i915/edp: Check supported link rates DPCD read

[Jani Nikula]

On Fri, 29 May 2026, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 29 May 2026, Nikita Zhandarovich <n.zhandarovich@fintech.ru> wrote:
>> intel_edp_set_sink_rates() reads DP_SUPPORTED_LINK_RATES into a local
>> stack array and then parses the array unconditionally. If the read
>> fails, the array contents are not valid and may result in bogus sink
>> link rates being used.
>>
>> Use drm_dp_dpcd_read_data() and clear the sink rate array on failure,
>> so the existing parser falls back to the default sink rate handling.
>>
>> Found by Linux Verification Center (linuxtesting.org) with static
>> analysis tool SVACE.
>>
>> Fixes: 68f357cb7347 ("drm/i915/dp: generate and cache sink rate array for all DP, not just eDP 1.4")
>> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

And pushed to drm-intel-next, thanks for the patch.

>
>> ---
>> v1 -> v2:
>> - Use drm_dp_dpcd_read_data() instead of drm_dp_dpcd_read().
>> - Avoid the goto by clearing sink_rates on read failure, as suggested by
>>   Jani Nikula.
>> - Adjust patch description.
>>
>>  drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 6ef2a0043cda..5c3e816b0135 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4678,10 +4678,17 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>>  
>>  	if (intel_dp->edp_dpcd[0] >= DP_EDP_14) {
>>  		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
>> +		int ret;
>>  		int i;
>>  
>> -		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
>> -				 sink_rates, sizeof(sink_rates));
>> +		ret = drm_dp_dpcd_read_data(&intel_dp->aux,
>> +					    DP_SUPPORTED_LINK_RATES,
>> +					    sink_rates, sizeof(sink_rates));
>> +		if (ret < 0) {
>> +			drm_dbg_kms(display->drm,
>> +				    "Unable to read eDP supported link rates, using default rates\n");
>> +			memset(sink_rates, 0, sizeof(sink_rates));
>> +		}
>>  
>>  		for (i = 0; i < ARRAY_SIZE(sink_rates); i++) {
>>  			int rate;

-- 
Jani Nikula, Intel

Claude review: drm/i915/edp: Check supported link rates DPCD read

[Claude Code Review Bot]

Overall Series Review

Subject: drm/i915/edp: Check supported link rates DPCD read
Author: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
Patches: 3
Reviewed: 2026-06-04T16:24:48.707404

---

This is a single-patch series (v2) that adds error checking to the `drm_dp_dpcd_read()` call in `intel_edp_set_sink_rates()`. The patch addresses a real static-analysis finding: the original code unconditionally parsed the `sink_rates[]` array even if the DPCD read failed, potentially using uninitialized stack data.

The fix is reasonable and follows the v2 feedback from Jani Nikula to use `drm_dp_dpcd_read_data()` instead of `drm_dp_dpcd_read()` and to avoid a goto. The approach of zeroing the array on failure is safe because the existing loop already treats a zero rate as a terminator (`if (rate == 0) break`), so `memset(sink_rates, 0, sizeof(sink_rates))` will cause the loop to immediately break and set `num_sink_rates = 0`, falling through to default handling.

**Verdict: The patch is correct and suitable for merging, with one minor observation below.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/i915/edp: Check supported link rates DPCD read

[Claude Code Review Bot]

Patch Review

**Correctness: Good**

The switch from `drm_dp_dpcd_read()` (returns bytes read or negative error) to `drm_dp_dpcd_read_data()` (returns 0 on success, negative on error, also handles short reads as `-EPROTO`) is the right call. It catches both outright failures *and* short reads, which the original code did not handle at all.

The `memset` fallback is elegant — it reuses the existing `rate == 0 → break` logic at line 4828 of the current tree:

```c
rate = le16_to_cpu(sink_rates[i]) * 200 / 10;
if (rate == 0)
    break;
```

This means `num_sink_rates` will be set to 0, which is the correct fallback.

**Minor observation:**

The `memset` on failure is technically redundant given `drm_dp_dpcd_read_data()`'s implementation. Looking at the function (drm_dp_helper.h:588-617), on the primary `drm_dp_dpcd_read()` failure path it falls through to byte-by-byte retry. If both paths fail, the buffer contents are indeterminate (partially written by the failed `drm_dp_dpcd_read()` call). So the `memset` is actually **necessary** for correctness — the buffer could contain partial/garbage data from a failed read. This is fine as-is.

**One nit:** The debug message says `"Unable to read eDP supported link rates, using default rates"` but whether "default rates" are actually used depends on what happens after `num_sink_rates` ends up as 0. Looking at the broader function (line 4845 in the tree), `num_sink_rates` is set to `i` which will be 0 after the zeroed array causes immediate break. The caller path does handle `num_sink_rates == 0` by falling back to DPCD-based rate calculation. So the message is accurate enough, though "falling back to DPCD-based rates" would be more precise. This is not worth a respin.

**Summary: Patch is correct, addresses a real (if unlikely) bug, and the implementation is clean. No blocking issues.**

---
Generated by Claude Code Patch Reviewer