The change referenced by the Fixes tag added ctx->lock protection around
struct vidi_context members related to EDID allocation and freeing, but
vidi_connection_ioctl() still checks ctx->connected separately from the
state update.
Two authenticated clients can therefore issue concurrent connect
requests. Both can observe ctx->connected as disconnected before either
one updates it, then both allocate a drm_edid and assign ctx->raw_edid.
The second assignment overwrites the first drm_edid pointer, leaking the
first allocation.
Keep the connection-state check and the ctx->raw_edid/ctx->connected
updates in the same critical section. If a concurrent request has already
changed the connection state by the time a newly allocated EDID is ready,
drop the new EDID before returning.
Fixes: 52b330799e2d ("drm/exynos: vidi: use ctx->lock to protect struct
vidi_context member variables related to memory alloc/free")
Signed-off-by: Guangshuo Li <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 33 ++++++++++++++----------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 67bbf9b8bc0e..bc0af9e6bc81 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -252,15 +252,6 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void
*data,
return -EINVAL;
}
- mutex_lock(&ctx->lock);
- if (ctx->connected == vidi->connection) {
- mutex_unlock(&ctx->lock);
- DRM_DEV_DEBUG_KMS(ctx->dev,
- "same connection request.\n");
- return -EINVAL;
- }
- mutex_unlock(&ctx->lock);
-
if (vidi->connection) {
const struct drm_edid *drm_edid;
const void __user *edid_userptr = u64_to_user_ptr(vidi->edid);
@@ -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);
+ 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);
} else {
/* with connection = 0, free raw_edid */
mutex_lock(&ctx->lock);
+ if (ctx->connected == vidi->connection) {
+ mutex_unlock(&ctx->lock);
+ DRM_DEV_DEBUG_KMS(ctx->dev,
+ "same connection request.\n");
+ return -EINVAL;
+ }
+
drm_edid_free(ctx->raw_edid);
ctx->raw_edid = NULL;
+ ctx->connected = vidi->connection;
mutex_unlock(&ctx->lock);
}
- mutex_lock(&ctx->lock);
- ctx->connected = vidi->connection;
- mutex_unlock(&ctx->lock);
-
drm_helper_hpd_irq_event(ctx->drm_dev);
return 0;
--
2.43.0
