Re: [PATCH 07/11] drm/i915/hdcp: switch to drm_dp_dpcd_{read_byte, read_data, write_data}()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/hdcp: incorrect buffer advancement in intel_dp_hdcp2_read_msg()
--

commit ef5e2443a19df2b1333678ad805fe64be928f4df
Author: Jani Nikula <jani.nikula@intel.com>

drm/i915/hdcp: switch to drm_dp_dpcd_{read_byte, read_data, write_data}()

This commit switches DPCD accesses to modern functions that return 0 on success
and negative error codes on failure. It simplifies error handling and converts
logging to use %pe and ERR_PTR().

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c

[ ... ]

> @@ -493,14 +493,13 @@ ssize_t get_receiver_id_list_rx_info(struct intel_connector *connector,
>  {
>  	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
>  	struct drm_dp_aux *aux = &dig_port->dp.aux;
> -	ssize_t ret;
>  	u8 *rx_info = byte;
> +	int ret;
>  
> -	ret = drm_dp_dpcd_read(aux,
> -			       DP_HDCP_2_2_REG_RXINFO_OFFSET,
> -			       (void *)rx_info, HDCP_2_2_RXINFO_LEN);
> -	if (ret != HDCP_2_2_RXINFO_LEN)
> -		return ret >= 0 ? -EIO : ret;
> +	ret = drm_dp_dpcd_read_data(aux, DP_HDCP_2_2_REG_RXINFO_OFFSET,
> +				    rx_info, HDCP_2_2_RXINFO_LEN);
> +	if (ret < 0)
> +		return ret;

[Severity: High]
Will this return value cause issues for downstream topology authentication?

Since drm_dp_dpcd_read_data() returns 0 on success (instead of the number of
bytes read), get_receiver_id_list_rx_info() now returns 0 on success.

Looking at the caller in intel_dp_hdcp2_read_msg(), it expects the return
value to be the number of bytes read so it can advance the buffer pointer:

	if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) {
		ret = get_receiver_id_list_rx_info(connector, &dev_cnt, byte);
		if (ret < 0)
			return ret;

		byte += ret;

Since ret is 0, the pointer won't advance, which could cause the subsequent
payload read to overwrite the rx_info field.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789547331.git.jani.nikula@intel.com?part=7