Re: [PATCH v5 38/38] drm/vkms: Add ConfigFS interface for connector parent and port_id

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The function `connector_parent_connector_allow_link` returns an uninitialized variable `ret` on success, which can cause state desynchronization and a Use-After-Free vulnerability.
--

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

drm/vkms: Add ConfigFS interface for connector parent and port_id

This commit adds configuration interfaces for a connector's parent and port_id
using configfs. It allows port_id to hold any u8 value and permits the parent
to contain at most one symlink to another connector.

> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index dd2a9e450150b..4f4ec99448042 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c

[ ... ]

> @@ -1512,6 +1555,63 @@ static const struct config_item_type connector_possible_encoders_group_type = {
>  	.ct_owner	= THIS_MODULE,
>  };
>  
> +static int connector_parent_connector_allow_link(struct config_item *src,
> +						 struct config_item *target)
> +{
> +	struct vkms_configfs_connector *connector;
> +	struct vkms_config_connector *connector_cfg;
> +	struct vkms_configfs_connector *parent;
> +	struct vkms_config_connector *parent_cfg;
> +	int ret;
> +
> +	if (target->ci_type != &connector_item_type)
> +		return -EINVAL;
> +
> +	connector = connector_parent_item_to_vkms_configfs_connector(src);
> +	connector_cfg = connector->config;
> +	parent = connector_item_to_vkms_configfs_connector(target);
> +	parent_cfg = parent->config;
> +
> +	if (connector->dev != parent->dev)
> +		return -EINVAL;
> +
> +	scoped_guard(mutex, &connector->dev->lock) {
> +		if (vkms_config_connector_get_parent(connector_cfg))
> +			return -EMLINK;
> +
> +		if (connector->dev->enabled && connector_cfg->enabled)
> +			return -EBUSY;
> +
> +		vkms_config_connector_attach_parent(connector_cfg,
> +						    parent_cfg);
> +	}
> +
> +	return ret;

[Severity: High]
Could this return an uninitialized value on the success path? 

In connector_parent_connector_allow_link(), ret is declared but never
initialized before being returned at the end of the function. 

If ret happens to contain a non-zero value from the stack, configfs will 
treat the link creation as a failure and abort creating the symlink. 
However, the internal VKMS state has already been modified to point to the 
target connector via vkms_config_connector_attach_parent(). 

If the user subsequently removes the target connector using rmdir, does 
this leave a dangling pointer in the VKMS configuration that could result 
in a use-after-free later on?

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