Skip to content

Commit 52b3307

Browse files
name2965daeinki
authored andcommitted
drm/exynos: vidi: use ctx->lock to protect struct vidi_context member variables related to memory alloc/free
Exynos Virtual Display driver performs memory alloc/free operations without lock protection, which easily causes concurrency problem. For example, use-after-free can occur in race scenario like this: ``` CPU0 CPU1 CPU2 ---- ---- ---- vidi_connection_ioctl() if (vidi->connection) // true drm_edid = drm_edid_alloc(); // alloc drm_edid ... ctx->raw_edid = drm_edid; ... drm_mode_getconnector() drm_helper_probe_single_connector_modes() vidi_get_modes() if (ctx->raw_edid) // true drm_edid_dup(ctx->raw_edid); if (!drm_edid) // false ... vidi_connection_ioctl() if (vidi->connection) // false drm_edid_free(ctx->raw_edid); // free drm_edid ... drm_edid_alloc(drm_edid->edid) kmemdup(edid); // UAF!! ... ``` To prevent these vulns, at least in vidi_context, member variables related to memory alloc/free should be protected with ctx->lock. Cc: <[email protected]> Signed-off-by: Jeongjun Park <[email protected]> Signed-off-by: Inki Dae <[email protected]>
1 parent d4c98c0 commit 52b3307

1 file changed

Lines changed: 32 additions & 6 deletions

File tree

drivers/gpu/drm/exynos/exynos_drm_vidi.c

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,29 +187,37 @@ static ssize_t vidi_store_connection(struct device *dev,
187187
const char *buf, size_t len)
188188
{
189189
struct vidi_context *ctx = dev_get_drvdata(dev);
190-
int ret;
190+
int ret, new_connected;
191191

192-
ret = kstrtoint(buf, 0, &ctx->connected);
192+
ret = kstrtoint(buf, 0, &new_connected);
193193
if (ret)
194194
return ret;
195-
196-
if (ctx->connected > 1)
195+
if (new_connected > 1)
197196
return -EINVAL;
198197

198+
mutex_lock(&ctx->lock);
199+
199200
/*
200201
* Use fake edid data for test. If raw_edid is set then it can't be
201202
* tested.
202203
*/
203204
if (ctx->raw_edid) {
204205
DRM_DEV_DEBUG_KMS(dev, "edid data is not fake data.\n");
205-
return -EINVAL;
206+
ret = -EINVAL;
207+
goto fail;
206208
}
207209

210+
ctx->connected = new_connected;
211+
mutex_unlock(&ctx->lock);
212+
208213
DRM_DEV_DEBUG_KMS(dev, "requested connection.\n");
209214

210215
drm_helper_hpd_irq_event(ctx->drm_dev);
211216

212217
return len;
218+
fail:
219+
mutex_unlock(&ctx->lock);
220+
return ret;
213221
}
214222

215223
static DEVICE_ATTR(connection, 0644, vidi_show_connection,
@@ -244,11 +252,14 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,
244252
return -EINVAL;
245253
}
246254

255+
mutex_lock(&ctx->lock);
247256
if (ctx->connected == vidi->connection) {
257+
mutex_unlock(&ctx->lock);
248258
DRM_DEV_DEBUG_KMS(ctx->dev,
249259
"same connection request.\n");
250260
return -EINVAL;
251261
}
262+
mutex_unlock(&ctx->lock);
252263

253264
if (vidi->connection) {
254265
const struct drm_edid *drm_edid;
@@ -282,14 +293,21 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,
282293
"edid data is invalid.\n");
283294
return -EINVAL;
284295
}
296+
mutex_lock(&ctx->lock);
285297
ctx->raw_edid = drm_edid;
298+
mutex_unlock(&ctx->lock);
286299
} else {
287300
/* with connection = 0, free raw_edid */
301+
mutex_lock(&ctx->lock);
288302
drm_edid_free(ctx->raw_edid);
289303
ctx->raw_edid = NULL;
304+
mutex_unlock(&ctx->lock);
290305
}
291306

307+
mutex_lock(&ctx->lock);
292308
ctx->connected = vidi->connection;
309+
mutex_unlock(&ctx->lock);
310+
293311
drm_helper_hpd_irq_event(ctx->drm_dev);
294312

295313
return 0;
@@ -304,7 +322,7 @@ static enum drm_connector_status vidi_detect(struct drm_connector *connector,
304322
* connection request would come from user side
305323
* to do hotplug through specific ioctl.
306324
*/
307-
return ctx->connected ? connector_status_connected :
325+
return READ_ONCE(ctx->connected) ? connector_status_connected :
308326
connector_status_disconnected;
309327
}
310328

@@ -327,11 +345,15 @@ static int vidi_get_modes(struct drm_connector *connector)
327345
const struct drm_edid *drm_edid;
328346
int count;
329347

348+
mutex_lock(&ctx->lock);
349+
330350
if (ctx->raw_edid)
331351
drm_edid = drm_edid_dup(ctx->raw_edid);
332352
else
333353
drm_edid = drm_edid_alloc(fake_edid_info, sizeof(fake_edid_info));
334354

355+
mutex_unlock(&ctx->lock);
356+
335357
drm_edid_connector_update(connector, drm_edid);
336358

337359
count = drm_edid_connector_add_modes(connector);
@@ -483,9 +505,13 @@ static void vidi_remove(struct platform_device *pdev)
483505
{
484506
struct vidi_context *ctx = platform_get_drvdata(pdev);
485507

508+
mutex_lock(&ctx->lock);
509+
486510
drm_edid_free(ctx->raw_edid);
487511
ctx->raw_edid = NULL;
488512

513+
mutex_unlock(&ctx->lock);
514+
489515
component_del(&pdev->dev, &vidi_component_ops);
490516
}
491517

0 commit comments

Comments
 (0)