[PATCH v2 2/3] drm/sysfs: Don't pollute connector->kdev if drm_sysfs_connector_add() fails

[ville.syrjala@xxxxxxxxxxxxxxx]

From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Currently if drm_sysfs_connector_add() fails, it can leave connector->kdev
populated with an ERR_PTR value, or pointing to an already freed device.
Use a temporary kdev pointer during drm_sysfs_connector_add(), and
only set connector->kdev if the function succeeds. This avoids oopsing
if drm_sysfs_connector_add() gets called for a connector where
drm_sysfs_connector_add() previously failed.

Give drm_sysfs_device_add() the same treatment for the sake of
consistency.

v2: s/drm_connector_sysfs_add/drm_sysfs_connector_add
    Make drm_sysfs_device_add() tolerate multiple calls

Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/drm_sysfs.c | 46 +++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 1a35ea5..95c701a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -370,6 +370,7 @@ static struct bin_attribute edid_attr = {
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
 	int attr_cnt = 0;
 	int opt_cnt = 0;
 	int i;
@@ -378,22 +379,22 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev = device_create(drm_class, dev->primary->kdev,
-					0, connector, "card%d-%s",
-					dev->primary->index, drm_get_connector_name(connector));
+	kdev = device_create(drm_class, dev->primary->kdev,
+			     0, connector, "card%d-%s",
+			     dev->primary->index, drm_get_connector_name(connector));
 	DRM_DEBUG("adding \"%s\" to sysfs\n",
 		  drm_get_connector_name(connector));
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		ret = PTR_ERR(connector->kdev);
+	if (IS_ERR(kdev)) {
+		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(kdev));
+		ret = PTR_ERR(kdev);
 		goto out;
 	}
 
 	/* Standard attributes */
 
 	for (attr_cnt = 0; attr_cnt < ARRAY_SIZE(connector_attrs); attr_cnt++) {
-		ret = device_create_file(connector->kdev, &connector_attrs[attr_cnt]);
+		ret = device_create_file(kdev, &connector_attrs[attr_cnt]);
 		if (ret)
 			goto err_out_files;
 	}
@@ -410,7 +411,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 		case DRM_MODE_CONNECTOR_Component:
 		case DRM_MODE_CONNECTOR_TV:
 			for (opt_cnt = 0; opt_cnt < ARRAY_SIZE(connector_attrs_opt1); opt_cnt++) {
-				ret = device_create_file(connector->kdev, &connector_attrs_opt1[opt_cnt]);
+				ret = device_create_file(kdev, &connector_attrs_opt1[opt_cnt]);
 				if (ret)
 					goto err_out_files;
 			}
@@ -419,21 +420,23 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 			break;
 	}
 
-	ret = sysfs_create_bin_file(&connector->kdev->kobj, &edid_attr);
+	ret = sysfs_create_bin_file(&kdev->kobj, &edid_attr);
 	if (ret)
 		goto err_out_files;
 
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	connector->kdev = kdev;
+
 	return 0;
 
 err_out_files:
 	for (i = 0; i < opt_cnt; i++)
-		device_remove_file(connector->kdev, &connector_attrs_opt1[i]);
+		device_remove_file(kdev, &connector_attrs_opt1[i]);
 	for (i = 0; i < attr_cnt; i++)
-		device_remove_file(connector->kdev, &connector_attrs[i]);
-	device_unregister(connector->kdev);
+		device_remove_file(kdev, &connector_attrs[i]);
+	device_unregister(kdev);
 
 out:
 	return ret;
@@ -501,6 +504,10 @@ EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 int drm_sysfs_device_add(struct drm_minor *minor)
 {
 	char *minor_str;
+	struct device *kdev;
+
+	if (minor->kdev)
+		return 0;
 
 	if (minor->type == DRM_MINOR_CONTROL)
 		minor_str = "controlD%d";
@@ -509,13 +516,16 @@ int drm_sysfs_device_add(struct drm_minor *minor)
         else
                 minor_str = "card%d";
 
-	minor->kdev = device_create(drm_class, minor->dev->dev,
-				    MKDEV(DRM_MAJOR, minor->index),
-				    minor, minor_str, minor->index);
-	if (IS_ERR(minor->kdev)) {
-		DRM_ERROR("device create failed %ld\n", PTR_ERR(minor->kdev));
-		return PTR_ERR(minor->kdev);
+	kdev = device_create(drm_class, minor->dev->dev,
+			     MKDEV(DRM_MAJOR, minor->index),
+			     minor, minor_str, minor->index);
+	if (IS_ERR(kdev)) {
+		DRM_ERROR("device create failed %ld\n", PTR_ERR(kdev));
+		return PTR_ERR(kdev);
 	}
+
+	minor->kdev = kdev;
+
 	return 0;
 }
 
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx