Hello Nicolas,

On the principle I agree with this patch, I just need to take time to properly think about lifetime of vkms_config vs connector and pointer validity to avoid use-after-free / null pointer dereference. I will try to review it next week or more probably just after the FOSDEM.

In the meantime, if you want to try / think about the possible issue: I think there will be a use-after-free if you unbind the driver using the sysfs interface and interract with configfs interface.

Thanks a lot for this series,
Louis Chauvet


On 1/23/26 20:44, Nicolas Frattaroli wrote:
From: Marius Vlad <[email protected]>

By passing the connector rather than the device to
vkms_trigger_connector_hotplug, vkms can trigger connector hotplugging
events that contain the connector ID.

Signed-off-by: Marius Vlad <[email protected]>
Signed-off-by: Nicolas Frattaroli <[email protected]>
---
  drivers/gpu/drm/vkms/vkms_configfs.c  | 2 +-
  drivers/gpu/drm/vkms/vkms_connector.c | 6 +++---
  drivers/gpu/drm/vkms/vkms_connector.h | 4 ++--
  3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c 
b/drivers/gpu/drm/vkms/vkms_configfs.c
index d6e203d59b45..63a27f671e6a 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -554,7 +554,7 @@ static ssize_t connector_status_store(struct config_item 
*item,
                vkms_config_connector_set_status(connector->config, status);
if (connector->dev->enabled && old_status != status)
-                       
vkms_trigger_connector_hotplug(connector->dev->config->dev);
+                       
vkms_trigger_connector_hotplug(connector->config->connector);

Here connector->config is valid, but connector->config->connector is probably invalid if the driver is unbind. We may need to add some refcount to avoid this kind of issue.

The other way around, I think there could be issue if the configfs folder is completly removed (that possible, there is no way to forbid deletions in configfs), the config object is freed but maybe used in the "DRM" part of VKMS (for connector status update maybe).

        }
return (ssize_t)count;
diff --git a/drivers/gpu/drm/vkms/vkms_connector.c 
b/drivers/gpu/drm/vkms/vkms_connector.c
index b0a6b212d3f4..cad64eff72ea 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.c
+++ b/drivers/gpu/drm/vkms/vkms_connector.c
@@ -88,9 +88,9 @@ struct vkms_connector *vkms_connector_init(struct vkms_device 
*vkmsdev)
        return connector;
  }
-void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev)
+void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector)
  {
-       struct drm_device *dev = &vkmsdev->drm;
+       struct drm_connector *connector = &vkms_connector->base;
- drm_kms_helper_hotplug_event(dev);
+       drm_kms_helper_connector_hotplug_event(connector);
  }
diff --git a/drivers/gpu/drm/vkms/vkms_connector.h 
b/drivers/gpu/drm/vkms/vkms_connector.h
index ed312f4eff3a..7cd76d01b10b 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.h
+++ b/drivers/gpu/drm/vkms/vkms_connector.h
@@ -28,8 +28,8 @@ struct vkms_connector *vkms_connector_init(struct vkms_device 
*vkmsdev);
/**
   * vkms_trigger_connector_hotplug() - Update the device's connectors status
- * @vkmsdev: VKMS device to update
+ * @vkms_connector: VKMS connector to update
   */
-void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev);
+void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector);
#endif /* _VKMS_CONNECTOR_H_ */


Reply via email to