Re: [PATCH] drm/exynos: vidi: fix EDID leak on concurrent connection ioctl

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent execution of `vidi_connection_ioctl` and device removal leads to a Use-After-Free of `ctx`.
--

--- Patch [1]: [PATCH] drm/exynos: vidi: fix EDID leak on concurrent connection ioctl ---
commit a672572794a1e16312bff77e391a1da1f3e7e9fe
Author: Guangshuo Li <lgs201920130244@gmail.com>

drm/exynos: vidi: fix EDID leak on concurrent connection ioctl

This commit fixes a memory leak in vidi_connection_ioctl() caused by
concurrent connection requests. It ensures the connection state check
and the EDID assignment are protected by the same critical section to
prevent a second request from overwriting an already allocated EDID.

> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 67bbf9b8bc0ef..bc0af9e6bc810 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c

[ ... ]

> @@ -293,21 +284,37 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,
>  					  "edid data is invalid.\n");
>  			return -EINVAL;
>  		}
> +
>  		mutex_lock(&ctx->lock);

[Severity: High]
This is a pre-existing issue, but can this result in a Use-After-Free if
the device is unbound concurrently?

Looking at the start of vidi_connection_ioctl(), ctx is resolved locklessly:

	struct device *dev = priv ? priv->vidi_dev : NULL;
	struct vidi_context *ctx = dev ? dev_get_drvdata(dev) : NULL;

If an authenticated client calls this ioctl, it can sleep earlier in this
function during copy_from_user() or drm_edid_alloc().

If the exynos-drm-vidi platform device is concurrently unbound via sysfs,
vidi_remove() could complete and devres may free the ctx structure while
the ioctl is sleeping.

Since Exynos DRM uses drm_dev_unregister() instead of drm_dev_unplug()
during teardown, in-flight ioctls are not blocked by the DRM core's
drm_dev_is_unplugged() check.

When the ioctl wakes up and reaches this mutex_lock(&ctx->lock), will it
dereference the freed ctx pointer?

> +		if (ctx->connected == vidi->connection) {
> +			mutex_unlock(&ctx->lock);
> +			drm_edid_free(drm_edid);
> +			DRM_DEV_DEBUG_KMS(ctx->dev,
> +					  "same connection request.\n");
> +			return -EINVAL;
> +		}
> +
> +		drm_edid_free(ctx->raw_edid);
> +
>  		ctx->raw_edid = drm_edid;
> +		ctx->connected = vidi->connection;
>  		mutex_unlock(&ctx->lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708032847.712897-1-lgs201920130244@gmail.com?part=1