I found these EDID/CEA patches sitting in a branch in my tree. I worked on them at some point, but forgot about them when something more urgent came up.
They haven't been tested all that well, mainly because currently there is no way to test this code, apart from trying on real hardware. If someone is looking for a good project, this would be one: 1. Split EDID parsing from the rest of the junk in drm_edid.c 2. Make it compile in user space 3. Cook up some regression and fuzz testing framework
From: Ville Syrjälä ville.syrjala@linux.intel.com
Make sure we don't access beyond the extension block when parsing CEA detailed timing descriptors.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/drm_edid.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5873e48..a346b04 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -551,6 +551,9 @@ cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) u8 d = ext[0x02]; u8 *det_base = ext + d;
+ if (d > 127) + return; + n = (127 - d) / 18; for (i = 0; i < n; i++) cb((struct detailed_timing *)(det_base + 18 * i), closure);
From: Ville Syrjälä ville.syrjala@linux.intel.com
Make sure drm_detect_hdmi_monitor() and drm_detect_monitor_audio() don't access beyond the extension block.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 99 ++++++++++++++++++++++++++++++++----------- 1 files changed, 73 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a346b04..aa28d1c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1502,16 +1502,57 @@ do_cea_modes (struct drm_connector *connector, u8 *db, u8 len) }
static int +cea_db_payload_len(const u8 *db) +{ + return db[0] & 0x1f; +} + +static int +cea_db_tag(const u8 *db) +{ + return db[0] >> 5; +} + +static int +cea_revision(const u8 *cea) +{ + return cea[1]; +} + +static int +cea_db_offsets(const u8 *cea, int *start, int *end) +{ + /* Data block offset in CEA extension block */ + *start = 4; + *end = cea[2]; + if (*end == 0) + *end = 127; + if (*end < 4 || *end > 127) + return -ERANGE; + return 0; +} + +#define for_each_cea_db(cea, i, start, end) \ + for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) + +static int add_cea_modes(struct drm_connector *connector, struct edid *edid) { u8 * cea = drm_find_cea_extension(edid); u8 * db, dbl; int modes = 0;
- if (cea && cea[1] >= 3) { - for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) { - dbl = db[0] & 0x1f; - if (((db[0] & 0xe0) >> 5) == VIDEO_BLOCK) + if (cea && cea_revision(cea) >= 3) { + int i, start, end; + + if (cea_db_offsets(cea, &start, &end)) + return 0; + + for_each_cea_db(cea, i, start, end) { + db = &cea[i]; + dbl = cea_db_payload_len(db); + + if (cea_db_tag(db) == VIDEO_BLOCK) modes += do_cea_modes (connector, db+1, dbl); } } @@ -1602,19 +1643,29 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) eld[18] = edid->prod_code[0]; eld[19] = edid->prod_code[1];
- if (cea[1] >= 3) - for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) { - dbl = db[0] & 0x1f; - - switch ((db[0] & 0xe0) >> 5) { + if (cea_revision(cea) >= 3) { + int i, start, end; + + if (cea_db_offsets(cea, &start, &end)) { + start = 0; + end = 0; + } + + for_each_cea_db(cea, i, start, end) { + db = &cea[i]; + dbl = cea_db_payload_len(db); + + switch (cea_db_tag(db)) { case AUDIO_BLOCK: /* Audio Data Block, contains SADs */ sad_count = dbl / 3; - memcpy(eld + 20 + mnl, &db[1], dbl); + if (dbl >= 1) + memcpy(eld + 20 + mnl, &db[1], dbl); break; case SPEAKER_BLOCK: - /* Speaker Allocation Data Block */ - eld[7] = db[1]; + /* Speaker Allocation Data Block */ + if (dbl >= 1) + eld[7] = db[1]; break; case VENDOR_BLOCK: /* HDMI Vendor-Specific Data Block */ @@ -1625,6 +1676,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) break; } } + } eld[5] |= sad_count << 4; eld[2] = (20 + mnl + sad_count * 3 + 3) / 4;
@@ -1710,19 +1762,16 @@ bool drm_detect_hdmi_monitor(struct edid *edid) if (!edid_ext) goto end;
- /* Data block offset in CEA extension block */ - start_offset = 4; - end_offset = edid_ext[2]; + if (cea_db_offsets(edid_ext, &start_offset, &end_offset)) + goto end;
/* * Because HDMI identifier is in Vendor Specific Block, * search it from all data blocks of CEA extension. */ - for (i = start_offset; i < end_offset; - /* Increased by data block len */ - i += ((edid_ext[i] & 0x1f) + 1)) { + for_each_cea_db(edid_ext, i, start_offset, end_offset) { /* Find vendor specific block */ - if ((edid_ext[i] >> 5) == VENDOR_BLOCK) { + if (cea_db_tag(&edid_ext[i]) == VENDOR_BLOCK) { hdmi_id = edid_ext[i + 1] | (edid_ext[i + 2] << 8) | edid_ext[i + 3] << 16; /* Find HDMI identifier */ @@ -1765,15 +1814,13 @@ bool drm_detect_monitor_audio(struct edid *edid) goto end; }
- /* Data block offset in CEA extension block */ - start_offset = 4; - end_offset = edid_ext[2]; + if (cea_db_offsets(edid_ext, &start_offset, &end_offset)) + goto end;
- for (i = start_offset; i < end_offset; - i += ((edid_ext[i] & 0x1f) + 1)) { - if ((edid_ext[i] >> 5) == AUDIO_BLOCK) { + for_each_cea_db(edid_ext, i, start_offset, end_offset) { + if (cea_db_tag(&edid_ext[i]) == AUDIO_BLOCK) { has_audio = true; - for (j = 1; j < (edid_ext[i] & 0x1f); j += 3) + for (j = 1; j < cea_db_payload_len(&edid_ext[i]) + 1; j += 3) DRM_DEBUG_KMS("CEA audio format %d\n", (edid_ext[i + j] >> 3) & 0xf); goto end;
From: Ville Syrjälä ville.syrjala@linux.intel.com
The length of HDMI VSDB must be at least 5 bytes. Other than the minimum, nothing else about the length is specified. Check the length before accessing any additional field beyond the minimum length. --- drivers/gpu/drm/drm_edid.c | 33 +++++++++++++++++++++------------ 1 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index aa28d1c..2611ba8 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1561,19 +1561,28 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) }
static void -parse_hdmi_vsdb(struct drm_connector *connector, uint8_t *db) +parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db) { - connector->eld[5] |= (db[6] >> 7) << 1; /* Supports_AI */ + u8 len = cea_db_payload_len(db);
- connector->dvi_dual = db[6] & 1; - connector->max_tmds_clock = db[7] * 5; - - connector->latency_present[0] = db[8] >> 7; - connector->latency_present[1] = (db[8] >> 6) & 1; - connector->video_latency[0] = db[9]; - connector->audio_latency[0] = db[10]; - connector->video_latency[1] = db[11]; - connector->audio_latency[1] = db[12]; + if (len >= 6) { + connector->eld[5] |= (db[6] >> 7) << 1; /* Supports_AI */ + connector->dvi_dual = db[6] & 1; + } + if (len >= 7) + connector->max_tmds_clock = db[7] * 5; + if (len >= 8) { + connector->latency_present[0] = db[8] >> 7; + connector->latency_present[1] = (db[8] >> 6) & 1; + } + if (len >= 9) + connector->video_latency[0] = db[9]; + if (len >= 10) + connector->audio_latency[0] = db[10]; + if (len >= 11) + connector->video_latency[1] = db[11]; + if (len >= 12) + connector->audio_latency[1] = db[12];
DRM_LOG_KMS("HDMI: DVI dual %d, " "max TMDS clock %d, " @@ -1669,7 +1678,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) break; case VENDOR_BLOCK: /* HDMI Vendor-Specific Data Block */ - if (db[1] == 0x03 && db[2] == 0x0c && db[3] == 0) + if (dbl >= 5 && db[1] == 0x03 && db[2] == 0x0c && db[3] == 0) parse_hdmi_vsdb(connector, db); break; default:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There are two slightly different pieces of code for HDMI VSDB detection. Unify the code into a single helper function.
Also fix a bug where drm_detect_hdmi_monitor() would stop looking for the HDMI VSDB after the first vendor specific block is found, whether or not that block happened to be the HDMI VSDB. The standard allows for any number of vendor specific blocks to be present.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 38 ++++++++++++++++++++++---------------- 1 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2611ba8..f5c9cea 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1606,6 +1606,21 @@ monitor_name(struct detailed_timing *t, void *data) *(u8 **)data = t->data.other_data.data.str.str; }
+static bool cea_db_is_hdmi_vsdb(const u8 *db) +{ + int hdmi_id; + + if (cea_db_tag(db) != VENDOR_BLOCK) + return false; + + if (cea_db_payload_len(db) < 5) + return false; + + hdmi_id = db[1] | (db[2] << 8) | (db[3] << 16); + + return hdmi_id == HDMI_IDENTIFIER; +} + /** * drm_edid_to_eld - build ELD from EDID * @connector: connector corresponding to the HDMI/DP sink @@ -1678,7 +1693,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) break; case VENDOR_BLOCK: /* HDMI Vendor-Specific Data Block */ - if (dbl >= 5 && db[1] == 0x03 && db[2] == 0x0c && db[3] == 0) + if (cea_db_is_hdmi_vsdb(db)) parse_hdmi_vsdb(connector, db); break; default: @@ -1763,35 +1778,26 @@ EXPORT_SYMBOL(drm_select_eld); bool drm_detect_hdmi_monitor(struct edid *edid) { u8 *edid_ext; - int i, hdmi_id; + int i; int start_offset, end_offset; - bool is_hdmi = false;
edid_ext = drm_find_cea_extension(edid); if (!edid_ext) - goto end; + return false;
if (cea_db_offsets(edid_ext, &start_offset, &end_offset)) - goto end; + return false;
/* * Because HDMI identifier is in Vendor Specific Block, * search it from all data blocks of CEA extension. */ for_each_cea_db(edid_ext, i, start_offset, end_offset) { - /* Find vendor specific block */ - if (cea_db_tag(&edid_ext[i]) == VENDOR_BLOCK) { - hdmi_id = edid_ext[i + 1] | (edid_ext[i + 2] << 8) | - edid_ext[i + 3] << 16; - /* Find HDMI identifier */ - if (hdmi_id == HDMI_IDENTIFIER) - is_hdmi = true; - break; - } + if (cea_db_is_hdmi_vsdb(&edid_ext[i])) + return true; }
-end: - return is_hdmi; + return false; } EXPORT_SYMBOL(drm_detect_hdmi_monitor);
dri-devel@lists.freedesktop.org
-
ville.syrjala＠linux.intel.com