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

Reply via email to