[PATCH] drm: Prevent use of uninitialised values whilst loading edid firmware

[Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>]

 CC      drivers/gpu/drm/drm_edid_load.o
drivers/gpu/drm/drm_edid_load.c: In function ‘drm_load_edid_firmware’: include/linux/err.h:39:17: warning: ‘edid’ may be used uninitialised in this function [-Wuninitialized]
drivers/gpu/drm/drm_edid_load.c:141:22: note: ‘edid’ was declared here

In the process, we can make the error handling more resilient.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/drm_edid_load.c |   75 +++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 271b42b..4b57a4c 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -136,59 +136,51 @@ static u8 generic_edid[GENERIC_EDIDS][128] = {
 static u8 *edid_load(struct drm_connector *connector, const char *name,
 			const char *connector_name)
 {
-	const struct firmware *fw;
+	const struct firmware *fw = NULL;
 	struct platform_device *pdev;
-	u8 *fwdata = NULL, *edid, *new_edid;
-	int fwsize, expected;
-	int builtin = 0, err = 0;
+	u8 *fwdata, *edid;
+	int fwsize, expected, err, 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 (!IS_ERR(pdev)) {
+		err = request_firmware(&fw, name, &pdev->dev);
+		platform_device_unregister(pdev);
+	} else
+		err = PTR_ERR(pdev);
 
-	if (err) {
+	if (err == 0) {
+		fwdata = (u8 *)fw->data;
+		fwsize = fw->size;
+		builtin = 0;
+	} else {
 		i = 0;
 		while (i < GENERIC_EDIDS && strcmp(name, generic_edid_name[i]))
 			i++;
-		if (i < GENERIC_EDIDS) {
-			err = 0;
-			builtin = 1;
-			fwdata = generic_edid[i];
-			fwsize = sizeof(generic_edid[i]);
+		if (i >= GENERIC_EDIDS) {
+			DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n",
+					name, err);
+			edid = ERR_PTR(err);
+			goto out;
 		}
-	}
 
-	if (err) {
-		DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n",
-		    name, err);
-		goto out;
-	}
-
-	if (fwdata == NULL) {
-		fwdata = (u8 *) fw->data;
-		fwsize = fw->size;
+		fwdata = generic_edid[i];
+		fwsize = sizeof(generic_edid[i]);
+		builtin = 1;
 	}
 
 	expected = (fwdata[0x7e] + 1) * EDID_LENGTH;
 	if (expected != fwsize) {
 		DRM_ERROR("Size of EDID firmware \"%s\" is invalid "
 		    "(expected %d, got %d)\n", name, expected, (int) fwsize);
-		err = -EINVAL;
+		edid = ERR_PTR(-EINVAL);
 		goto relfw_out;
 	}
 
 	edid = kmemdup(fwdata, fwsize, GFP_KERNEL);
 	if (edid == NULL) {
-		err = -ENOMEM;
+		edid = ERR_PTR(-ENOMEM);
 		goto relfw_out;
 	}
 
@@ -197,7 +189,7 @@ 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;
+		edid = ERR_PTR(-EINVAL);
 		goto relfw_out;
 	}
 
@@ -210,19 +202,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 "
@@ -231,12 +222,10 @@ static u8 *edid_load(struct drm_connector *connector, const char *name,
 	    name, connector_name);
 
 relfw_out:
-	release_firmware(fw);
+	if (fw)
+		release_firmware(fw);
 
 out:
-	if (err)
-		return ERR_PTR(err);
-
 	return edid;
 }
 
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel