| Message ID | 1379427251-1457-1-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On Tue, Sep 17, 2013 at 06:15:31PM +0300, Jani Nikula wrote: > 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> Is that for both patches? I've merged the first one for now to dinq ... Thanks, Daniel
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;