Re: [PATCH 2/8] drm/edid: Don't accept any old garbage as a display descriptor

On Fri, Jan 24, 2020 at 3:02 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

Currently we assume any 18 byte descriptor to be a display descritor if only the tag byte matches the expected value. But for detailed timing descriptors that same byte is just the lower 8 bits of hblank, and as such can match any display descriptor tag. To properly validate that the 18 byte descriptor is in fact a display descriptor we must also examine bytes 0-2 (just byte 1 should actually suffice but the spec does say that bytes 0 and 2 must also always be zero for display descriptors so we check those too).

Unlike Allen's original proposed patch to just fix is_rb() we roll this out across the board to fix everything.

Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

Acked-by: Alex Deucher alexander.deucher@amd.com

---

drivers/gpu/drm/drm_edid.c | 65 ++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1b6e544cf5c7..96ae1fde4ce2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2196,6 +2196,12 @@ struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, } EXPORT_SYMBOL(drm_mode_find_dmt);

+static bool is_display_descriptor(const u8 d[18], u8 tag) +{

•   return d[0] == 0x00 && d[1] == 0x00 &&
  

•           d[2] == 0x00 && d[3] == tag;
  

+}

typedef void detailed_cb(struct detailed_timing *timing, void *closure);

static void @@ -2257,9 +2263,12 @@ static void is_rb(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;

•   if (r[3] == EDID_DETAIL_MONITOR_RANGE)
  

•           if (r[15] & 0x10)
  

•                   *(bool *)data = true;
  

•   if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))
  

•           return;
  

•   if (r[15] & 0x10)
  

•           *(bool *)data = true;
  

}

/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ @@ -2279,7 +2288,11 @@ static void find_gtf2(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;

•   if (r[3] == EDID_DETAIL_MONITOR_RANGE && r[10] == 0x02)
  

•   if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))
  

•           return;
  

•   if (r[10] == 0x02)
            *(u8 **)data = r;
  

}

@@ -2818,7 +2831,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c) struct detailed_non_pixel *data = &timing->data.other_data; struct detailed_data_monitor_range *range = &data->data.range;

•   if (data->type != EDID_DETAIL_MONITOR_RANGE)
  

•   if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_MONITOR_RANGE))
            return;
  
    closure->modes += drm_dmt_modes_for_range(closure->connector,
  

@@ -2897,10 +2910,11 @@ static void do_established_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;

•   struct detailed_non_pixel *data = &timing->data.other_data;
  
  

•   if (data->type == EDID_DETAIL_EST_TIMINGS)
  

•           closure->modes += drm_est3_modes(closure->connector, timing);
  

•   if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_EST_TIMINGS))
  

•           return;
  

•   closure->modes += drm_est3_modes(closure->connector, timing);
  

}

/** @@ -2949,19 +2963,19 @@ do_standard_modes(struct detailed_timing *timing, void *c) struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector; struct edid *edid = closure->edid;

•   int i;
  
  

•   if (data->type == EDID_DETAIL_STD_MODES) {
  

•           int i;
  

•           for (i = 0; i < 6; i++) {
  

•                   struct std_timing *std;
  

•                   struct drm_display_mode *newmode;
  

•   if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES))
  

•           return;
  
  

•                   std = &data->data.timings[i];
  

•                   newmode = drm_mode_std(connector, edid, std);
  

•                   if (newmode) {
  

•                           drm_mode_probed_add(connector, newmode);
  

•                           closure->modes++;
  

•                   }
  

•   for (i = 0; i < 6; i++) {
  

•           struct std_timing *std = &data->data.timings[i];
  

•           struct drm_display_mode *newmode;
  

•           newmode = drm_mode_std(connector, edid, std);
  

•           if (newmode) {
  

•                   drm_mode_probed_add(connector, newmode);
  

•                   closure->modes++;
            }
    }
  

} @@ -3056,10 +3070,11 @@ static void do_cvt_mode(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;

•   struct detailed_non_pixel *data = &timing->data.other_data;
  
  

•   if (data->type == EDID_DETAIL_CVT_3BYTE)
  

•           closure->modes += drm_cvt_modes(closure->connector, timing);
  

•   if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_CVT_3BYTE))
  

•           return;
  

•   closure->modes += drm_cvt_modes(closure->connector, timing);
  

}

static int @@ -4285,8 +4300,10 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) static void monitor_name(struct detailed_timing *t, void *data) {

•   if (t->data.other_data.type == EDID_DETAIL_MONITOR_NAME)
  

•           *(u8 **)data = t->data.other_data.data.str.str;
  

•   if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME))
  

•           return;
  

•   *(u8 **)data = t->data.other_data.data.str.str;
  

}

static int get_monitor_name(struct edid *edid, char name[13])

2.24.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel