Re: [PATCH v5 30/38] drm/vkms: Introduce config for connector supported colorspace

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Missing support for DRM_MODE_CONNECTOR_HDMIB in VKMS connector initialization.

Pre-existing issues:
- [High] Lockless iteration over configuration lists in debugfs allows concurrent Use-After-Free if configfs items are removed.
--

commit adab925d90477ebe9d202328861dc983f52ebccf
Author: Louis Chauvet <louis.chauvet@bootlin.com>

drm/vkms: Introduce config for connector supported colorspace

This commit introduces the ability to configure supported colorspaces
for HDMI, eDP, and DP connectors in VKMS to emulate HDR features.

> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index c407aea0bc6a9..ea9bc5d06be67 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c

[ ... ]

> @@ -600,6 +600,10 @@ static int vkms_config_show(struct seq_file *m, void *data)
>  			   drm_get_connector_status_name(vkms_config_connector_get_status(connector_cfg)));
>  		seq_printf(m, "\ttype=%s\n",
>  			   drm_get_connector_type_name(vkms_config_connector_get_type(connector_cfg)));
> +		seq_puts(m, "\tsupported colorspaces=");
> +		show_bitfield(m, vkms_config_connector_get_supported_colorspaces(connector_cfg),
> +			      drm_get_colorspace_name);

[Severity: High]
This is a pre-existing issue, but does this lockless iteration over the
connectors list in vkms_config_show() expose us to a use-after-free?

If userspace removes a connector via configfs concurrently,
vkms_config_destroy_connector() will free the connector_cfg without any locks
synchronizing with this debugfs read path. This patch adds additional reads
of the connector_cfg during this iteration.

> +		seq_puts(m, "\n");
>  	}
>  
>  	return 0;

[ ... ]

> diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
> index 5b29dcade7e19..a1f08167def1d 100644
> --- a/drivers/gpu/drm/vkms/vkms_connector.c
> +++ b/drivers/gpu/drm/vkms/vkms_connector.c

[ ... ]

> +static int __must_check vkms_connector_init(struct vkms_connector *connector,
> +					    struct vkms_config_connector *connector_cfg)
> +{
> +	int ret = 0;
> +
> +	if (vkms_config_connector_get_supported_colorspaces(connector_cfg)) {
> +		if (connector_cfg->type == DRM_MODE_CONNECTOR_HDMIA) {
> +			ret = drm_mode_create_hdmi_colorspace_property(&connector->base,
> +								       vkms_config_connector_get_supported_colorspaces(connector_cfg));

[Severity: Medium]
Does this logic inadvertently omit DRM_MODE_CONNECTOR_HDMIB? 

If a user configures a connector as HDMIB with supported colorspaces via
configfs, it looks like the colorspace property will not be created or
attached since only HDMIA is checked during this initialization.

> +			if (ret)
> +				return ret;
> +		} else if (connector_cfg->type == DRM_MODE_CONNECTOR_DisplayPort ||
> +			   connector_cfg->type == DRM_MODE_CONNECTOR_eDP) {
> +			ret = drm_mode_create_dp_colorspace_property(&connector->base,
> +								     vkms_config_connector_get_supported_colorspaces(connector_cfg));
> +			if (ret)
> +				return ret;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627-vkms-all-config-v5-0-854aa0840926@bootlin.com?part=30