If the firmware is builtin and userspace is not yet running, we can stall the boot process for a minute whilst the firmware loader times out probing for a non-existent connector EDID. This is contrary to the expectations of providing a builtin EDID!

In the process, we can rearrange the code to make the error handling more resilient and prevent gcc warning about unitialised variables along the error paths.

v2: Load builtins first, fix gcc second (Jani) and cosmetics (Ville). v3: Verify that we do not read beyond the end of the fwdata (Ville) v4: Restore the lost parenthesis (Jani)

Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Jani Nikula jani.nikula@linux.intel.com Reviewed-by: Jani Nikula jani.nikula@linux.intel.com --- drivers/gpu/drm/drm_edid_load.c | 108 ++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 271b42bbfb72..75f204bbb888 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -32,7 +32,7 @@ MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use specified EDID blob " "from built-in data or /lib/firmware instead. ");

#define GENERIC_EDIDS 5 -static char *generic_edid_name[GENERIC_EDIDS] = { +static const char *generic_edid_name[GENERIC_EDIDS] = { "edid/1024x768.bin", "edid/1280x1024.bin", "edid/1600x1200.bin", @@ -40,7 +40,7 @@ static char *generic_edid_name[GENERIC_EDIDS] = { "edid/1920x1080.bin", };

-static u8 generic_edid[GENERIC_EDIDS][128] = { +static const u8 generic_edid[GENERIC_EDIDS][128] = { { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -133,63 +133,68 @@ static u8 generic_edid[GENERIC_EDIDS][128] = { }, };

+static int edid_size(const u8 *edid, int data_size) +{ + if (data_size < EDID_LENGTH) + return 0; + + return (edid[0x7e] + 1) * EDID_LENGTH; +} + static u8 *edid_load(struct drm_connector *connector, const char *name, const char *connector_name) { - const struct firmware *fw; - struct platform_device *pdev; - u8 *fwdata = NULL, *edid, *new_edid; - int fwsize, expected; - int builtin = 0, err = 0; + const struct firmware *fw = NULL; + const u8 *fwdata; + u8 *edid; + int fwsize, builtin; int i, valid_extensions = 0; bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);

- pdev = platform_device_register_simple(connector_name, -1, NULL, 0); - if (IS_ERR(pdev)) { - DRM_ERROR("Failed to register EDID firmware platform device " - "for connector "%s"\n", connector_name); - err = -EINVAL; - goto out; - } - - err = request_firmware(&fw, name, &pdev->dev); - platform_device_unregister(pdev); - - if (err) { - i = 0; - while (i < GENERIC_EDIDS && strcmp(name, generic_edid_name[i])) - i++; - if (i < GENERIC_EDIDS) { - err = 0; - builtin = 1; + builtin = 0; + for (i = 0; i < GENERIC_EDIDS; i++) { + if (strcmp(name, generic_edid_name[i]) == 0) { fwdata = generic_edid[i]; fwsize = sizeof(generic_edid[i]); + builtin = 1; + break; } } + if (!builtin) { + struct platform_device *pdev; + int err;

- if (err) { - DRM_ERROR("Requesting EDID firmware "%s" failed (err=%d)\n", - name, err); - goto out; - } + pdev = platform_device_register_simple(connector_name, -1, NULL, 0); + if (IS_ERR(pdev)) { + DRM_ERROR("Failed to register EDID firmware platform device " + "for connector "%s"\n", connector_name); + return ERR_CAST(pdev); + } + + err = request_firmware(&fw, name, &pdev->dev); + platform_device_unregister(pdev); + if (err) { + DRM_ERROR("Requesting EDID firmware "%s" failed (err=%d)\n", + name, err); + return ERR_PTR(err); + }

- if (fwdata == NULL) { - fwdata = (u8 *) fw->data; + fwdata = fw->data; fwsize = fw->size; }

- expected = (fwdata[0x7e] + 1) * EDID_LENGTH; - if (expected != fwsize) { + if (edid_size(fwdata, fwsize) != fwsize) { DRM_ERROR("Size of EDID firmware "%s" is invalid " - "(expected %d, got %d)\n", name, expected, (int) fwsize); - err = -EINVAL; - goto relfw_out; + "(expected %d, got %d)\n", name, + edid_size(fwdata, fwsize), (int)fwsize); + edid = ERR_PTR(-EINVAL); + goto out; }

edid = kmemdup(fwdata, fwsize, GFP_KERNEL); if (edid == NULL) { - err = -ENOMEM; - goto relfw_out; + edid = ERR_PTR(-ENOMEM); + goto out; }

if (!drm_edid_block_valid(edid, 0, print_bad_edid)) { @@ -197,8 +202,8 @@ static u8 *edid_load(struct drm_connector *connector, const char *name, DRM_ERROR("Base block of EDID firmware "%s" is invalid ", name); kfree(edid); - err = -EINVAL; - goto relfw_out; + edid = ERR_PTR(-EINVAL); + goto out; }

for (i = 1; i <= edid[0x7e]; i++) { @@ -210,19 +215,18 @@ static u8 *edid_load(struct drm_connector *connector, const char *name, }

if (valid_extensions != edid[0x7e]) { + u8 *new_edid; + edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data " ""%s" for connector "%s"\n", valid_extensions, edid[0x7e], name, connector_name); edid[0x7e] = valid_extensions; + new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH, - GFP_KERNEL); - if (new_edid == NULL) { - err = -ENOMEM; - kfree(edid); - goto relfw_out; - } - edid = new_edid; + GFP_KERNEL); + if (new_edid) + edid = new_edid; }

DRM_INFO("Got %s EDID base block and %d extension%s from " @@ -230,13 +234,9 @@ static u8 *edid_load(struct drm_connector *connector, const char *name, "external", valid_extensions, valid_extensions == 1 ? "" : "s", name, connector_name);

-relfw_out: - release_firmware(fw); - out: - if (err) - return ERR_PTR(err); - + if (fw) + release_firmware(fw); return edid; }