From: Danny Feng <dfeng@redhat.com> Date: Fri, 23 Oct 2009 06:00:40 -0400 Subject: [drm] r128: check for init on all ioctls that require it Message-id: 20091023100101.12183.65369.sendpatchset@danny O-Subject: [PATCH RHEL5.5] CVE-2009-3620 drm/r128: Add test for initialisation to all ioctls that require it Bugzilla: 529603 RH-Acked-by: Chuck Ebbert <cebbert@redhat.com> CVE: CVE-2009-3620 RHBZ#(CVE-2009-3620): https://bugzilla.redhat.com/show_bug.cgi?id=529603 Description: Almost all r128's private ioctls require that the CCE state has already been initialised. However, most do not test that this has been done, and will proceed to dereference a null pointer. This may result in a security vulnerability, since some ioctls are unprivileged. Upstream status: commit 7dc482 Brew build: https://brewweb.devel.redhat.com/taskinfo?taskID=2043583 KABI: no harm diff --git a/drivers/char/drm/r128_cce.c b/drivers/char/drm/r128_cce.c index db5a604..554052a 100644 --- a/drivers/char/drm/r128_cce.c +++ b/drivers/char/drm/r128_cce.c @@ -353,6 +353,11 @@ static int r128_do_init_cce(drm_device_t * dev, drm_r128_init_t * init) DRM_DEBUG("\n"); + if (dev->dev_private) { + DRM_DEBUG("called when already initialized\n"); + return DRM_ERR(EINVAL); + } + dev_priv = drm_alloc(sizeof(drm_r128_private_t), DRM_MEM_DRIVER); if (dev_priv == NULL) return DRM_ERR(ENOMEM); @@ -655,6 +660,8 @@ int r128_cce_start(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + if (dev_priv->cce_running || dev_priv->cce_mode == R128_PM4_NONPM4) { DRM_DEBUG("%s while CCE running\n", __FUNCTION__); return 0; @@ -678,6 +685,8 @@ int r128_cce_stop(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + DRM_COPY_FROM_USER_IOCTL(stop, (drm_r128_cce_stop_t __user *) data, sizeof(stop)); @@ -719,10 +728,7 @@ int r128_cce_reset(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); - if (!dev_priv) { - DRM_DEBUG("%s called before init done\n", __FUNCTION__); - return DRM_ERR(EINVAL); - } + DEV_INIT_TEST_WITH_RETURN(dev_priv); r128_do_cce_reset(dev_priv); @@ -740,6 +746,8 @@ int r128_cce_idle(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + if (dev_priv->cce_running) { r128_do_cce_flush(dev_priv); } @@ -754,6 +762,8 @@ int r128_engine_reset(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev->dev_private); + return r128_do_engine_reset(dev); } diff --git a/drivers/char/drm/r128_drv.h b/drivers/char/drm/r128_drv.h index 94abffb..4d91472 100644 --- a/drivers/char/drm/r128_drv.h +++ b/drivers/char/drm/r128_drv.h @@ -414,6 +414,14 @@ static __inline__ void r128_update_ring_snapshot(drm_r128_private_t * dev_priv) * Misc helper macros */ +#define DEV_INIT_TEST_WITH_RETURN(_dev_priv) \ +do { \ + if (!_dev_priv) { \ + DRM_ERROR("called with no initialization\n"); \ + return DRM_ERR(EINVAL); \ + } \ +} while (0) + #define RING_SPACE_TEST_WITH_RETURN( dev_priv ) \ do { \ drm_r128_ring_buffer_t *ring = &dev_priv->ring; int i; \ diff --git a/drivers/char/drm/r128_state.c b/drivers/char/drm/r128_state.c index a080cdd..bc2a693 100644 --- a/drivers/char/drm/r128_state.c +++ b/drivers/char/drm/r128_state.c @@ -1244,12 +1244,16 @@ static int r128_cce_clear(DRM_IOCTL_ARGS) { DRM_DEVICE; drm_r128_private_t *dev_priv = dev->dev_private; - drm_r128_sarea_t *sarea_priv = dev_priv->sarea_priv; + drm_r128_sarea_t *sarea_priv; drm_r128_clear_t clear; DRM_DEBUG("\n"); LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + + sarea_priv = dev_priv->sarea_priv; + DRM_COPY_FROM_USER_IOCTL(clear, (drm_r128_clear_t __user *) data, sizeof(clear)); @@ -1316,6 +1320,8 @@ static int r128_cce_flip(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + RING_SPACE_TEST_WITH_RETURN(dev_priv); if (!dev_priv->page_flipping) @@ -1336,6 +1342,8 @@ static int r128_cce_swap(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + RING_SPACE_TEST_WITH_RETURN(dev_priv); if (sarea_priv->nbox > R128_NR_SAREA_CLIPRECTS) @@ -1360,10 +1368,7 @@ static int r128_cce_vertex(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); - if (!dev_priv) { - DRM_ERROR("%s called with no initialization\n", __FUNCTION__); - return DRM_ERR(EINVAL); - } + DEV_INIT_TEST_WITH_RETURN(dev_priv); DRM_COPY_FROM_USER_IOCTL(vertex, (drm_r128_vertex_t __user *) data, sizeof(vertex)); @@ -1420,10 +1425,7 @@ static int r128_cce_indices(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); - if (!dev_priv) { - DRM_ERROR("%s called with no initialization\n", __FUNCTION__); - return DRM_ERR(EINVAL); - } + DEV_INIT_TEST_WITH_RETURN(dev_priv); DRM_COPY_FROM_USER_IOCTL(elts, (drm_r128_indices_t __user *) data, sizeof(elts)); @@ -1489,6 +1491,8 @@ static int r128_cce_blit(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + DRM_COPY_FROM_USER_IOCTL(blit, (drm_r128_blit_t __user *) data, sizeof(blit)); @@ -1518,6 +1522,8 @@ static int r128_cce_depth(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + DRM_COPY_FROM_USER_IOCTL(depth, (drm_r128_depth_t __user *) data, sizeof(depth)); @@ -1552,6 +1558,8 @@ static int r128_cce_stipple(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); + DEV_INIT_TEST_WITH_RETURN(dev_priv); + DRM_COPY_FROM_USER_IOCTL(stipple, (drm_r128_stipple_t __user *) data, sizeof(stipple)); @@ -1580,10 +1588,7 @@ static int r128_cce_indirect(DRM_IOCTL_ARGS) LOCK_TEST_WITH_RETURN(dev, filp); - if (!dev_priv) { - DRM_ERROR("%s called with no initialization\n", __FUNCTION__); - return DRM_ERR(EINVAL); - } + DEV_INIT_TEST_WITH_RETURN(dev_priv); DRM_COPY_FROM_USER_IOCTL(indirect, (drm_r128_indirect_t __user *) data, sizeof(indirect)); @@ -1648,10 +1653,7 @@ static int r128_getparam(DRM_IOCTL_ARGS) drm_r128_getparam_t param; int value; - if (!dev_priv) { - DRM_ERROR("%s called with no initialization\n", __FUNCTION__); - return DRM_ERR(EINVAL); - } + DEV_INIT_TEST_WITH_RETURN(dev_priv); DRM_COPY_FROM_USER_IOCTL(param, (drm_r128_getparam_t __user *) data, sizeof(param));