Re: [PATCH 1/2] drm/i915: WARN is the DP aux read or write is too big

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

On Tue, 17 Sep 2013, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> So far we control everything and nothing exceeds the current limits,
> but (i) we never think about these limits when reviewing patches, (ii)
> not all the callers check the return values and (iii) if we ever hit
> any of these messages, we'll have to fix the code that added the bad
> message.
>
> The current limit for these messages is 20 since we only have 5 data
> registers on all the current gens.
>
> The checks inside intel_dp_aux_native_{write,read} are to prevent
> buffer overflows. The check inside intel_dp_aux_ch is to prevent
> writing past our 5 data registers.

I wish there were fewer magic values, but it does what it says on the
box.

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

> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8c70a83..c324a59 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -436,6 +436,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		goto out;
>  	}
>  
> +	/* Only 5 data registers! */
> +	if (WARN_ON(send_bytes > 20 || recv_size > 20)) {
> +		ret = -E2BIG;
> +		goto out;
> +	}
> +
>  	while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
>  		/* Must try at least 3 times according to DP spec */
>  		for (try = 0; try < 5; try++) {
> @@ -526,9 +532,10 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
>  	int msg_bytes;
>  	uint8_t	ack;
>  
> +	if (WARN_ON(send_bytes > 16))
> +		return -E2BIG;
> +
>  	intel_dp_check_edp(intel_dp);
> -	if (send_bytes > 16)
> -		return -1;
>  	msg[0] = AUX_NATIVE_WRITE << 4;
>  	msg[1] = address >> 8;
>  	msg[2] = address & 0xff;
> @@ -569,6 +576,9 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
>  	uint8_t ack;
>  	int ret;
>  
> +	if (WARN_ON(recv_bytes > 19))
> +		return -E2BIG;
> +
>  	intel_dp_check_edp(intel_dp);
>  	msg[0] = AUX_NATIVE_READ << 4;
>  	msg[1] = address >> 8;
> -- 
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx