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

Jani Nikula
2013-09-17T15:18:15Z

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