Message ID | 20200124200231.10517-5-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/8] drm/edid: Check the number of detailed timing descriptors in the CEA ext block | expand |
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > After much head scratching I managed to convince myself that > for_each_displayid_db() has already done the bounds checks for > the DispID CEA data block. Which is why we don't need to repeat > them in cea_db_offsets(). To avoid having to go through that > pain again in the future add a comment which explains this fact. > > Cc: Andres Rodriguez <andresx7@gmail.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 3df5744026b0..0369a54e3d32 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) > * no non-DTD data. > */ > if (cea[0] == DATA_BLOCK_CTA) { > + /* > + * for_each_displayid_db() has already verified > + * that these stay within expected bounds. > + */ I think the preferred format is to have the start of the comment be on the first line after the /* with that fixed: Acked-by: Alex Deucher <alexander.deucher@amd.com> > *start = 3; > *end = *start + cea[2]; > } else if (cea[0] == CEA_EXT) { > -- > 2.24.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Jan 27, 2020 at 05:30:42PM -0500, Alex Deucher wrote: > On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala > <ville.syrjala@linux.intel.com> wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > After much head scratching I managed to convince myself that > > for_each_displayid_db() has already done the bounds checks for > > the DispID CEA data block. Which is why we don't need to repeat > > them in cea_db_offsets(). To avoid having to go through that > > pain again in the future add a comment which explains this fact. > > > > Cc: Andres Rodriguez <andresx7@gmail.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/drm_edid.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 3df5744026b0..0369a54e3d32 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) > > * no non-DTD data. > > */ > > if (cea[0] == DATA_BLOCK_CTA) { > > + /* > > + * for_each_displayid_db() has already verified > > + * that these stay within expected bounds. > > + */ > > I think the preferred format is to have the start of the comment be on > the first line after the /* with that fixed: Nope. > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > > *start = 3; > > *end = *start + cea[2]; > > } else if (cea[0] == CEA_EXT) { > > -- > > 2.24.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> -----Original Message----- > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjälä > Sent: Tuesday, January 28, 2020 5:14 PM > To: Alex Deucher <alexdeucher@gmail.com> > Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; Andres Rodriguez > <andresx7@gmail.com>; Maling list - DRI developers <dri- > devel@lists.freedesktop.org> > Subject: Re: [PATCH 5/8] drm/edid: Document why we don't bounds check the > DispID CEA block start/end > > On Mon, Jan 27, 2020 at 05:30:42PM -0500, Alex Deucher wrote: > > On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala > > <ville.syrjala@linux.intel.com> wrote: > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > After much head scratching I managed to convince myself that > > > for_each_displayid_db() has already done the bounds checks for the > > > DispID CEA data block. Which is why we don't need to repeat them in > > > cea_db_offsets(). To avoid having to go through that pain again in > > > the future add a comment which explains this fact. > > > > > > Cc: Andres Rodriguez <andresx7@gmail.com> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/drm_edid.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > > index 3df5744026b0..0369a54e3d32 100644 > > > --- a/drivers/gpu/drm/drm_edid.c > > > +++ b/drivers/gpu/drm/drm_edid.c > > > @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) > > > * no non-DTD data. > > > */ > > > if (cea[0] == DATA_BLOCK_CTA) { > > > + /* > > > + * for_each_displayid_db() has already verified > > > + * that these stay within expected bounds. > > > + */ > > > > I think the preferred format is to have the start of the comment be on > > the first line after the /* with that fixed: > > Nope. Yes the style is correct here, comment is apt as well. Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > > > > *start = 3; > > > *end = *start + cea[2]; > > > } else if (cea[0] == CEA_EXT) { > > > -- > > > 2.24.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3df5744026b0..0369a54e3d32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) * no non-DTD data. */ if (cea[0] == DATA_BLOCK_CTA) { + /* + * for_each_displayid_db() has already verified + * that these stay within expected bounds. + */ *start = 3; *end = *start + cea[2]; } else if (cea[0] == CEA_EXT) {