On Thursday, December 23, 2021 1:02 PM, Ville Syrjälä wrote: >On Sun, Dec 12, 2021 at 11:33:31PM +0800, Lee Shawn C wrote: >> According to CEA-861-F chapter 7.5.4. It says "The VSDB shall contain >> the >> 3 bytes of the IEEE OUI as well as any additional payload bytes needed." >> Now DRM driver check HDMI OUI but VSDB payload size at least five bytes. >> That may caused some HDMI monitors' audio feature can't be enabled. >> Because of they only have three bytes payload (OUI only) in VSDB. >> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Adam Jackson <ajax@xxxxxxxxxx> >> Cc: Dave Airlie <airlied@xxxxxxxxxx> >> Signed-off-by: Lee Shawn C <shawn.c.lee@xxxxxxxxx> >> --- >> drivers/gpu/drm/drm_edid.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 12893e7be89b..5aa4a6bf4a13 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -4205,7 +4205,7 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db) >> if (cea_db_tag(db) != VENDOR_BLOCK) >> return false; >> >> - if (cea_db_payload_len(db) < 5) >> + if (cea_db_payload_len(db) < 3) >> return false; > >I was a a bit worried that we are now assuming that we can parse some >stuff without further length checks, but looks like that's just the >"source physical address" stuff which we do not parse in drm_edid.c, >and the other fields we do parse are actually optional and so already >have the require length checks. So this seems fine. > Thanks for the comments! We will share the information to customer. Best regards, Shawn >Closes: https://gitlab.freedesktop.org/drm/misc/-/issues/28 >Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >Note that there is a second edid parser in cec_get_edid_spa_location() >that does parse the source physical address. You may want to double >check that it doesn't make any bad assumptions about the length >of the vsdb either. I think we should probably get rid of that >second parser and just have drm_edid.c extract the source physical >address and pass that on directly to the cec code instead. But I >guess the cec code still couldn't remove the second parser >since some media drivers need it :( Though it could then perhaps >be moved into the media code instead of having it as a massive >inline function in the cec headers. > >-- >Ville Syrjälä >Intel
