| Message ID | 20220330170426.349248-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/edid: fix invalid EDID extension block filtering | expand |
On Wed, Mar 30, 2022 at 08:04:26PM +0300, Jani Nikula wrote: > The invalid EDID block filtering uses the number of valid EDID > extensions instead of all EDID extensions for looping the extensions in > the copy. This is fine, by coincidence, if all the invalid blocks are at > the end of the EDID. However, it's completely broken if there are > invalid extensions in the middle; the invalid blocks are included and > valid blocks are excluded. > > Fix it by modifying the base block after, not before, the copy. > > Fixes: 14544d0937bf ("drm/edid: Only print the bad edid when aborting") > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index d79b06f7f34c..8829120470ab 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2031,9 +2031,6 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, > > connector_bad_edid(connector, edid, edid[0x7e] + 1); > > - edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; > - edid[0x7e] = valid_extensions; > - > new = kmalloc_array(valid_extensions + 1, EDID_LENGTH, > GFP_KERNEL); > if (!new) > @@ -2050,6 +2047,9 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, > base += EDID_LENGTH; > } > > + new[EDID_LENGTH - 1] += new[0x7e] - valid_extensions; > + new[0x7e] = valid_extensions; > + > kfree(edid); > edid = new; > } > -- > 2.30.2
On Wed, 30 Mar 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Mar 30, 2022 at 08:04:26PM +0300, Jani Nikula wrote: >> The invalid EDID block filtering uses the number of valid EDID >> extensions instead of all EDID extensions for looping the extensions in >> the copy. This is fine, by coincidence, if all the invalid blocks are at >> the end of the EDID. However, it's completely broken if there are >> invalid extensions in the middle; the invalid blocks are included and >> valid blocks are excluded. >> >> Fix it by modifying the base block after, not before, the copy. >> >> Fixes: 14544d0937bf ("drm/edid: Only print the bad edid when aborting") >> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks, pushed to drm-misc-next. I didn't bother with -fixes, because the bug is ancient and rare, and that would only cause conflicts for me. BR, Jani. > >> --- >> drivers/gpu/drm/drm_edid.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index d79b06f7f34c..8829120470ab 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -2031,9 +2031,6 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, >> >> connector_bad_edid(connector, edid, edid[0x7e] + 1); >> >> - edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; >> - edid[0x7e] = valid_extensions; >> - >> new = kmalloc_array(valid_extensions + 1, EDID_LENGTH, >> GFP_KERNEL); >> if (!new) >> @@ -2050,6 +2047,9 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, >> base += EDID_LENGTH; >> } >> >> + new[EDID_LENGTH - 1] += new[0x7e] - valid_extensions; >> + new[0x7e] = valid_extensions; >> + >> kfree(edid); >> edid = new; >> } >> -- >> 2.30.2
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d79b06f7f34c..8829120470ab 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2031,9 +2031,6 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, connector_bad_edid(connector, edid, edid[0x7e] + 1); - edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; - edid[0x7e] = valid_extensions; - new = kmalloc_array(valid_extensions + 1, EDID_LENGTH, GFP_KERNEL); if (!new) @@ -2050,6 +2047,9 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, base += EDID_LENGTH; } + new[EDID_LENGTH - 1] += new[0x7e] - valid_extensions; + new[0x7e] = valid_extensions; + kfree(edid); edid = new; }
The invalid EDID block filtering uses the number of valid EDID extensions instead of all EDID extensions for looping the extensions in the copy. This is fine, by coincidence, if all the invalid blocks are at the end of the EDID. However, it's completely broken if there are invalid extensions in the middle; the invalid blocks are included and valid blocks are excluded. Fix it by modifying the base block after, not before, the copy. Fixes: 14544d0937bf ("drm/edid: Only print the bad edid when aborting") Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_edid.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)