Re: [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug

[Louis Chauvet <louis.chauvet@bootlin.com>]

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 <marius.vlad@collabora.com>
> 
> 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 <marius.vlad@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>   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_ */
>