Unverified Commit 52b33079 authored by Jeongjun Park's avatar Jeongjun Park Committed by Inki Dae
Browse files

drm/exynos: vidi: use ctx->lock to protect struct vidi_context member... 


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: <stable@vger.kernel.org>
Signed-off-by: default avatarJeongjun Park <aha310510@gmail.com>
Signed-off-by: default avatarInki Dae <inki.dae@samsung.com>
parent d4c98c07

drivers/gpu/drm/exynos/exynos_drm_vidi.c

+32 −6
Original line number Diff line number Diff line
@@ -187,29 +187,37 @@ static ssize_t vidi_store_connection(struct device *dev, 
				const char *buf, size_t len)

{

	struct vidi_context *ctx = dev_get_drvdata(dev);

	int ret;

	int ret, new_connected;



	ret = kstrtoint(buf, 0, &ctx->connected);

	ret = kstrtoint(buf, 0, &new_connected);

	if (ret)

		return ret;



	if (ctx->connected > 1)

	if (new_connected > 1)

		return -EINVAL;



	mutex_lock(&ctx->lock);



	/*

	 * Use fake edid data for test. If raw_edid is set then it can't be

	 * tested.

	 */

	if (ctx->raw_edid) {

		DRM_DEV_DEBUG_KMS(dev, "edid data is not fake data.\n");

		return -EINVAL;

		ret = -EINVAL;

		goto fail;

	}



	ctx->connected = new_connected;

	mutex_unlock(&ctx->lock);



	DRM_DEV_DEBUG_KMS(dev, "requested connection.\n");



	drm_helper_hpd_irq_event(ctx->drm_dev);



	return len;

fail:

	mutex_unlock(&ctx->lock);

	return ret;

}



static DEVICE_ATTR(connection, 0644, vidi_show_connection,
@@ -244,11 +252,14 @@ 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;
@@ -282,14 +293,21 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data, 
					  "edid data is invalid.\n");

			return -EINVAL;

		}

		mutex_lock(&ctx->lock);

		ctx->raw_edid = drm_edid;

		mutex_unlock(&ctx->lock);

	} else {

		/* with connection = 0, free raw_edid */

		mutex_lock(&ctx->lock);

		drm_edid_free(ctx->raw_edid);

		ctx->raw_edid = NULL;

		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;
@@ -304,7 +322,7 @@ static enum drm_connector_status vidi_detect(struct drm_connector *connector, 
	 * connection request would come from user side

	 * to do hotplug through specific ioctl.

	 */

	return ctx->connected ? connector_status_connected :

	return READ_ONCE(ctx->connected) ? connector_status_connected :

			connector_status_disconnected;

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

	int count;



	mutex_lock(&ctx->lock);



	if (ctx->raw_edid)

		drm_edid = drm_edid_dup(ctx->raw_edid);

	else

		drm_edid = drm_edid_alloc(fake_edid_info, sizeof(fake_edid_info));



	mutex_unlock(&ctx->lock);



	drm_edid_connector_update(connector, drm_edid);



	count = drm_edid_connector_add_modes(connector);
@@ -483,9 +505,13 @@ static void vidi_remove(struct platform_device *pdev) 
{

	struct vidi_context *ctx = platform_get_drvdata(pdev);



	mutex_lock(&ctx->lock);



	drm_edid_free(ctx->raw_edid);

	ctx->raw_edid = NULL;



	mutex_unlock(&ctx->lock);



	component_del(&pdev->dev, &vidi_component_ops);

}