[PATCH v2] drm: Optimize tv properties creation

Edward Adam Davis <[email protected]>
Newsgroups org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
When adding gud properties for drm connector within the function
gud_connector_add_properties(), if the TV modes property is not added
first, drm_mode_create_tv_properties_legacy() would fail to add the TV
modes property because the tv_select_subconnector_property has already
been added. Other properties (such as brightness, contrast, etc.) are
affected by the same issue.

This causes gud_connector_property_lookup() to fail when looking for
the TV modes property (returning NULL), which subsequently triggers
issue [1] when a NULL property is passed to drm_object_attach_property().

The fix ensures that within drm_mode_create_tv_properties_legacy(), the
TV modes property is correctly added regardless of whether the subconnector
property exists.

Recreation of properties brightness(contrast, flicker reduction, overscan,
saturation, hue) must be prevented.

[1]
Oops: general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
RIP: 0010:drm_object_attach_property+0x85/0x3b0 drivers/gpu/drm/drm_mode_object.c:240
Call Trace:
 gud_connector_add_properties drivers/gpu/drm/gud/gud_connector.c:572 [inline]
 gud_connector_create drivers/gpu/drm/gud/gud_connector.c:680 [inline]
 gud_get_connectors+0x86e/0x1700 drivers/gpu/drm/gud/gud_connector.c:717
 gud_probe+0x17aa/0x1c20 drivers/gpu/drm/gud/gud_drv.c:635

Fixes: f453ba046074 ("DRM: add mode setting support")
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
Tested-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
---
v1 -> v2: avoid recreate brightness/contrast/.../hue properties;
	  update subject and comments

 drivers/gpu/drm/drm_connector.c | 106 ++++++++++++++++++--------------
 1 file changed, 60 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 11646453aaac..800fcc44e3f4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2175,29 +2175,30 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	struct drm_property *tv_subconnector;
 	unsigned int i;
 
-	if (dev->mode_config.tv_select_subconnector_property)
-		return 0;
-
-	/*
-	 * Basic connector properties
-	 */
-	tv_selector = drm_property_create_enum(dev, 0,
-					  "select subconnector",
-					  drm_tv_select_enum_list,
-					  ARRAY_SIZE(drm_tv_select_enum_list));
-	if (!tv_selector)
-		goto nomem;
+	if (!dev->mode_config.tv_select_subconnector_property) {
+		/*
+		 * Basic connector properties
+		 */
+		tv_selector = drm_property_create_enum(dev, 0,
+					"select subconnector",
+					drm_tv_select_enum_list,
+					ARRAY_SIZE(drm_tv_select_enum_list));
+		if (!tv_selector)
+			goto nomem;
 
-	dev->mode_config.tv_select_subconnector_property = tv_selector;
+		dev->mode_config.tv_select_subconnector_property = tv_selector;
+	}
 
-	tv_subconnector =
-		drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
-				    "subconnector",
-				    drm_tv_subconnector_enum_list,
-				    ARRAY_SIZE(drm_tv_subconnector_enum_list));
-	if (!tv_subconnector)
-		goto nomem;
-	dev->mode_config.tv_subconnector_property = tv_subconnector;
+	if (!dev->mode_config.tv_subconnector_property) {
+		tv_subconnector =
+			drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
+				"subconnector",
+				drm_tv_subconnector_enum_list,
+				ARRAY_SIZE(drm_tv_subconnector_enum_list));
+		if (!tv_subconnector)
+			goto nomem;
+		dev->mode_config.tv_subconnector_property = tv_subconnector;
+	}
 
 	/*
 	 * Other, TV specific properties: margins & TV modes.
@@ -2205,7 +2206,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	if (drm_mode_create_tv_margin_properties(dev))
 		goto nomem;
 
-	if (num_modes) {
+	if (num_modes && !dev->mode_config.legacy_tv_mode_property) {
 		dev->mode_config.legacy_tv_mode_property =
 			drm_property_create(dev, DRM_MODE_PROP_ENUM,
 					    "mode", num_modes);
@@ -2217,35 +2218,48 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 					      i, modes[i]);
 	}
 
-	dev->mode_config.tv_brightness_property =
-		drm_property_create_range(dev, 0, "brightness", 0, 100);
-	if (!dev->mode_config.tv_brightness_property)
-		goto nomem;
+	if (!dev->mode_config.tv_brightness_property) {
+		dev->mode_config.tv_brightness_property =
+			drm_property_create_range(dev, 0, "brightness", 0, 100);
+		if (!dev->mode_config.tv_brightness_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_contrast_property =
-		drm_property_create_range(dev, 0, "contrast", 0, 100);
-	if (!dev->mode_config.tv_contrast_property)
-		goto nomem;
+	if (!dev->mode_config.tv_contrast_property) {
+		dev->mode_config.tv_contrast_property =
+			drm_property_create_range(dev, 0, "contrast", 0, 100);
+		if (!dev->mode_config.tv_contrast_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_flicker_reduction_property =
-		drm_property_create_range(dev, 0, "flicker reduction", 0, 100);
-	if (!dev->mode_config.tv_flicker_reduction_property)
-		goto nomem;
+	if (!dev->mode_config.tv_flicker_reduction_property) {
+		dev->mode_config.tv_flicker_reduction_property =
+			drm_property_create_range(dev, 0, "flicker reduction",
+					0, 100);
+		if (!dev->mode_config.tv_flicker_reduction_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_overscan_property =
-		drm_property_create_range(dev, 0, "overscan", 0, 100);
-	if (!dev->mode_config.tv_overscan_property)
-		goto nomem;
+	if (!dev->mode_config.tv_overscan_property) {
+		dev->mode_config.tv_overscan_property =
+			drm_property_create_range(dev, 0, "overscan", 0, 100);
+		if (!dev->mode_config.tv_overscan_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_saturation_property =
-		drm_property_create_range(dev, 0, "saturation", 0, 100);
-	if (!dev->mode_config.tv_saturation_property)
-		goto nomem;
+	if (!dev->mode_config.tv_saturation_property) {
+		dev->mode_config.tv_saturation_property =
+			drm_property_create_range(dev, 0, "saturation", 0, 100);
+		if (!dev->mode_config.tv_saturation_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_hue_property =
-		drm_property_create_range(dev, 0, "hue", 0, 100);
-	if (!dev->mode_config.tv_hue_property)
-		goto nomem;
+	if (!dev->mode_config.tv_hue_property) {
+		dev->mode_config.tv_hue_property =
+			drm_property_create_range(dev, 0, "hue", 0, 100);
+		if (!dev->mode_config.tv_hue_property)
+			goto nomem;
+	}
 
 	return 0;
 nomem:
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.