Skip to content

Commit 3cc9398

Browse files
committed
Merge tag 'exynos-drm-next-for-v6.20' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next
Fix three regressions . Fix a regression where vidi_connection_ioctl() used the wrong device to look up the vidi context. It stores the vidi device in exynos_drm_private and uses it in ioctl(), preventing invalid pointer access and related bugs. . Fix a security regression where vidi_connection_ioctl() directly dereferenced a user pointer for EDID data. It copies EDID from user space with copy_from_user() into kernel memory before use, preventing arbitrary kernel memory access. . Fix a concurrency regression where vidi_context members related to EDID memory were accessed without locking. It protects alloc/free and state updates with ctx->lock, preventing race conditions and use-after-free bugs. Signed-off-by: Dave Airlie <[email protected]> From: Inki Dae <[email protected]> Link: https://patch.msgid.link/[email protected]
2 parents a60f627 + 52b3307 commit 3cc9398

2 files changed

Lines changed: 64 additions & 11 deletions

File tree

drivers/gpu/drm/exynos/exynos_drm_drv.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ struct drm_exynos_file_private {
199199
struct exynos_drm_private {
200200
struct device *g2d_dev;
201201
struct device *dma_dev;
202+
struct device *vidi_dev;
202203
void *mapping;
203204

204205
/* for atomic commit */

drivers/gpu/drm/exynos/exynos_drm_vidi.c

Lines changed: 63 additions & 11 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,
@@ -224,9 +232,14 @@ ATTRIBUTE_GROUPS(vidi);
224232
int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,
225233
struct drm_file *file_priv)
226234
{
227-
struct vidi_context *ctx = dev_get_drvdata(drm_dev->dev);
235+
struct exynos_drm_private *priv = drm_dev->dev_private;
236+
struct device *dev = priv ? priv->vidi_dev : NULL;
237+
struct vidi_context *ctx = dev ? dev_get_drvdata(dev) : NULL;
228238
struct drm_exynos_vidi_connection *vidi = data;
229239

240+
if (!ctx)
241+
return -ENODEV;
242+
230243
if (!vidi) {
231244
DRM_DEV_DEBUG_KMS(ctx->dev,
232245
"user data for vidi is null.\n");
@@ -239,21 +252,38 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,
239252
return -EINVAL;
240253
}
241254

255+
mutex_lock(&ctx->lock);
242256
if (ctx->connected == vidi->connection) {
257+
mutex_unlock(&ctx->lock);
243258
DRM_DEV_DEBUG_KMS(ctx->dev,
244259
"same connection request.\n");
245260
return -EINVAL;
246261
}
262+
mutex_unlock(&ctx->lock);
247263

248264
if (vidi->connection) {
249265
const struct drm_edid *drm_edid;
250-
const struct edid *raw_edid;
266+
const void __user *edid_userptr = u64_to_user_ptr(vidi->edid);
267+
void *edid_buf;
268+
struct edid hdr;
251269
size_t size;
252270

253-
raw_edid = (const struct edid *)(unsigned long)vidi->edid;
254-
size = (raw_edid->extensions + 1) * EDID_LENGTH;
271+
if (copy_from_user(&hdr, edid_userptr, sizeof(hdr)))
272+
return -EFAULT;
273+
274+
size = (hdr.extensions + 1) * EDID_LENGTH;
275+
276+
edid_buf = kmalloc(size, GFP_KERNEL);
277+
if (!edid_buf)
278+
return -ENOMEM;
255279

256-
drm_edid = drm_edid_alloc(raw_edid, size);
280+
if (copy_from_user(edid_buf, edid_userptr, size)) {
281+
kfree(edid_buf);
282+
return -EFAULT;
283+
}
284+
285+
drm_edid = drm_edid_alloc(edid_buf, size);
286+
kfree(edid_buf);
257287
if (!drm_edid)
258288
return -ENOMEM;
259289

@@ -263,14 +293,21 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,
263293
"edid data is invalid.\n");
264294
return -EINVAL;
265295
}
296+
mutex_lock(&ctx->lock);
266297
ctx->raw_edid = drm_edid;
298+
mutex_unlock(&ctx->lock);
267299
} else {
268300
/* with connection = 0, free raw_edid */
301+
mutex_lock(&ctx->lock);
269302
drm_edid_free(ctx->raw_edid);
270303
ctx->raw_edid = NULL;
304+
mutex_unlock(&ctx->lock);
271305
}
272306

307+
mutex_lock(&ctx->lock);
273308
ctx->connected = vidi->connection;
309+
mutex_unlock(&ctx->lock);
310+
274311
drm_helper_hpd_irq_event(ctx->drm_dev);
275312

276313
return 0;
@@ -285,7 +322,7 @@ static enum drm_connector_status vidi_detect(struct drm_connector *connector,
285322
* connection request would come from user side
286323
* to do hotplug through specific ioctl.
287324
*/
288-
return ctx->connected ? connector_status_connected :
325+
return READ_ONCE(ctx->connected) ? connector_status_connected :
289326
connector_status_disconnected;
290327
}
291328

@@ -308,11 +345,15 @@ static int vidi_get_modes(struct drm_connector *connector)
308345
const struct drm_edid *drm_edid;
309346
int count;
310347

348+
mutex_lock(&ctx->lock);
349+
311350
if (ctx->raw_edid)
312351
drm_edid = drm_edid_dup(ctx->raw_edid);
313352
else
314353
drm_edid = drm_edid_alloc(fake_edid_info, sizeof(fake_edid_info));
315354

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

318359
count = drm_edid_connector_add_modes(connector);
@@ -372,13 +413,16 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
372413
{
373414
struct vidi_context *ctx = dev_get_drvdata(dev);
374415
struct drm_device *drm_dev = data;
416+
struct exynos_drm_private *priv = drm_dev->dev_private;
375417
struct drm_encoder *encoder = &ctx->encoder;
376418
struct exynos_drm_plane *exynos_plane;
377419
struct exynos_drm_plane_config plane_config = { 0 };
378420
unsigned int i;
379421
int ret;
380422

381423
ctx->drm_dev = drm_dev;
424+
if (priv)
425+
priv->vidi_dev = dev;
382426

383427
plane_config.pixel_formats = formats;
384428
plane_config.num_pixel_formats = ARRAY_SIZE(formats);
@@ -424,8 +468,12 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
424468
static void vidi_unbind(struct device *dev, struct device *master, void *data)
425469
{
426470
struct vidi_context *ctx = dev_get_drvdata(dev);
471+
struct drm_device *drm_dev = data;
472+
struct exynos_drm_private *priv = drm_dev->dev_private;
427473

428474
timer_delete_sync(&ctx->timer);
475+
if (priv)
476+
priv->vidi_dev = NULL;
429477
}
430478

431479
static const struct component_ops vidi_component_ops = {
@@ -457,9 +505,13 @@ static void vidi_remove(struct platform_device *pdev)
457505
{
458506
struct vidi_context *ctx = platform_get_drvdata(pdev);
459507

508+
mutex_lock(&ctx->lock);
509+
460510
drm_edid_free(ctx->raw_edid);
461511
ctx->raw_edid = NULL;
462512

513+
mutex_unlock(&ctx->lock);
514+
463515
component_del(&pdev->dev, &vidi_component_ops);
464516
}
465517

0 commit comments

Comments
 (0)