drm/exynos: vidi: fix various memory corruption bugs
Subsystems (2)
cover drm/exynos: vidi: fix various memory corruption bugs
This backport patch should have been backported along with commit 52b330799e2d ("drm/exynos: vidi: use ctx->lock to protect struct vidi_context member variables related to memory alloc/free"), but was written separately because some commits were missing.
https://lore.kernel.org/all/[email protected]/
After this patch is backported, we plan to write additional patches to backport to the remaining longterm kernels.
Jeongjun Park (2):
drm/exynos: vidi: use priv->vidi_dev for ctx lookup in vidi_connection_ioctl() drm/exynos: vidi: fix to avoid directly dereferencing user pointer
drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + drivers/gpu/drm/exynos/exynos_drm_vidi.c | 36 +++++++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 5 deletions(-)
PATCH 1/2 drm/exynos: vidi: use priv->vidi_dev for ctx lookup in vidi_connection_ioctl()
[ Upstream commit d3968a0d85b211e197f2f4f06268a7031079e0d0 ]
vidi_connection_ioctl() retrieves the driver_data from drm_dev->dev to obtain a struct vidi_context pointer. However, drm_dev->dev is the exynos-drm master device, and the driver_data contained therein is not the vidi component device, but a completely different device.
This can lead to various bugs, ranging from null pointer dereferences and garbage value accesses to, in unlucky cases, out-of-bounds errors, use-after-free errors, and more.
To resolve this issue, we need to store/delete the vidi device pointer in exynos_drm_private->vidi_dev during bind/unbind, and then read this exynos_drm_private->vidi_dev within ioctl() to obtain the correct struct vidi_context pointer.
--- drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + drivers/gpu/drm/exynos/exynos_drm_vidi.c | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)
drivers/gpu/drm/exynos/exynos_drm_drv.h +1 -0
@@ -199,6 +199,7 @@ struct drm_exynos_file_private {struct exynos_drm_private {struct device *g2d_dev;struct device *dma_dev; +struct device *vidi_dev;void *mapping;/* for atomic commit */
drivers/gpu/drm/exynos/exynos_drm_vidi.c +13 -1
@@ -223,9 +223,14 @@ ATTRIBUTE_GROUPS(vidi);int vidi_connection_ioctl (struct drm_device *drm_dev,void *data,struct drm_file *file_priv) { -struct vidi_context *ctx =dev_get_drvdata (drm_dev->dev); +struct exynos_drm_private *priv = drm_dev->dev_private; +struct device *dev = priv ? priv->vidi_dev :NULL ; +struct vidi_context *ctx = dev ?dev_get_drvdata (dev) :NULL ;struct drm_exynos_vidi_connection *vidi = data; +if (!ctx) +return -ENODEV ; +if (!vidi) {DRM_DEV_DEBUG_KMS (ctx->dev,"user data for vidi is null.\n" );
@@ -371,6 +376,7 @@ static int vidi_bind(struct device *dev, struct device *master, void *data) {struct vidi_context *ctx =dev_get_drvdata (dev);struct drm_device *drm_dev = data; +struct exynos_drm_private *priv = drm_dev->dev_private;struct drm_encoder *encoder = &ctx->encoder;struct exynos_drm_plane *exynos_plane;struct exynos_drm_plane_config plane_config = {0 };
@@ -378,6 +384,8 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)int ret; ctx->drm_dev = drm_dev; +if (priv) + priv->vidi_dev = dev; plane_config.pixel_formats = formats; plane_config.num_pixel_formats =ARRAY_SIZE (formats);
@@ -423,8 +431,12 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)static void vidi_unbind (struct device *dev,struct device *master,void *data) {struct vidi_context *ctx =dev_get_drvdata (dev); +struct drm_device *drm_dev = data; +struct exynos_drm_private *priv = drm_dev->dev_private;timer_delete_sync (&ctx->timer); +if (priv) + priv->vidi_dev =NULL ; }static const struct component_ops vidi_component_ops = {
--
PATCH 2/2 drm/exynos: vidi: fix to avoid directly dereferencing user pointer
In reply to Jeongjun Park[ Upstream commit d4c98c077c7fb2dfdece7d605e694b5ea2665085 ]
In vidi_connection_ioctl(), vidi->edid(user pointer) is directly dereferenced in the kernel.
This allows arbitrary kernel memory access from the user space, so instead of directly accessing the user pointer in the kernel, we should modify it to copy edid to kernel memory using copy_from_user() and use it.
--- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
drivers/gpu/drm/exynos/exynos_drm_vidi.c +18 -4
@@ -251,13 +251,27 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data,if (vidi->connection) {const struct drm_edid *drm_edid; -const struct edid *raw_edid; +const void __user *edid_userptr =u64_to_user_ptr (vidi->edid); +void *edid_buf; +struct edid hdr;size_t size; - raw_edid = (const struct edid *)(unsigned long )vidi->edid; - size = (raw_edid->extensions +1 ) *EDID_LENGTH ; +if (copy_from_user (&hdr, edid_userptr,sizeof (hdr))) +return -EFAULT ; - drm_edid =drm_edid_alloc (raw_edid, size); + size = (hdr.extensions +1 ) *EDID_LENGTH ; + + edid_buf =kmalloc (size,GFP_KERNEL ); +if (!edid_buf) +return -ENOMEM ; + +if (copy_from_user (edid_buf, edid_userptr, size)) { +kfree (edid_buf); +return -EFAULT ; + } + + drm_edid =drm_edid_alloc (edid_buf, size); +kfree (edid_buf);if (!drm_edid)return -ENOMEM ;
--