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 <[email protected]> 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/[email protected]?part=1
