Skip to content

Commit

Permalink
drm/r128: Add test for initialisation to all ioctls that require it
Browse files Browse the repository at this point in the history 
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.

This adds a macro for the common initialisation test and changes all
ioctl implementations that require prior initialisation to use that
macro.

Also, r128_do_init_cce() does not test that the CCE state has not
been initialised already.  Repeated initialisation may lead to a crash
or resource leak.  This adds that test.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Dave Airlie <airlied@redhat.com>
  • Loading branch information
Ben Hutchings authored and Dave Airlie committed Aug 30, 2009
1 parent 70967ab commit 7dc482d
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 21 deletions.
18 changes: 14 additions & 4 deletions drivers/gpu/drm/r128/r128_cce.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,11 @@ static int r128_do_init_cce(struct drm_device * dev, drm_r128_init_t * init)

DRM_DEBUG("\n");

if (dev->dev_private) {
DRM_DEBUG("called when already initialized\n");
return -EINVAL;
}

dev_priv = kzalloc(sizeof(drm_r128_private_t), GFP_KERNEL);
if (dev_priv == NULL)
return -ENOMEM;
Expand Down Expand Up @@ -647,6 +652,8 @@ int r128_cce_start(struct drm_device *dev, void *data, struct drm_file *file_pri

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

if (dev_priv->cce_running || dev_priv->cce_mode == R128_PM4_NONPM4) {
DRM_DEBUG("while CCE running\n");
return 0;
Expand All @@ -669,6 +676,8 @@ int r128_cce_stop(struct drm_device *dev, void *data, struct drm_file *file_priv

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

/* Flush any pending CCE commands. This ensures any outstanding
* commands are exectuted by the engine before we turn it off.
*/
Expand Down Expand Up @@ -706,10 +715,7 @@ int r128_cce_reset(struct drm_device *dev, void *data, struct drm_file *file_pri

LOCK_TEST_WITH_RETURN(dev, file_priv);

if (!dev_priv) {
DRM_DEBUG("called before init done\n");
return -EINVAL;
}
DEV_INIT_TEST_WITH_RETURN(dev_priv);

r128_do_cce_reset(dev_priv);

Expand All @@ -726,6 +732,8 @@ int r128_cce_idle(struct drm_device *dev, void *data, struct drm_file *file_priv

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

if (dev_priv->cce_running) {
r128_do_cce_flush(dev_priv);
}
Expand All @@ -739,6 +747,8 @@ int r128_engine_reset(struct drm_device *dev, void *data, struct drm_file *file_

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev->dev_private);

return r128_do_engine_reset(dev);
}

Expand Down
8 changes: 8 additions & 0 deletions drivers/gpu/drm/r128/r128_drv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,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 -EINVAL; \
} \
} while (0)

#define RING_SPACE_TEST_WITH_RETURN( dev_priv ) \
do { \
drm_r128_ring_buffer_t *ring = &dev_priv->ring; int i; \
Expand Down
36 changes: 19 additions & 17 deletions drivers/gpu/drm/r128/r128_state.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1244,14 +1244,18 @@ static void r128_cce_dispatch_stipple(struct drm_device * dev, u32 * stipple)
static int r128_cce_clear(struct drm_device *dev, void *data, struct drm_file *file_priv)
{
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 = data;
DRM_DEBUG("\n");

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

RING_SPACE_TEST_WITH_RETURN(dev_priv);

sarea_priv = dev_priv->sarea_priv;

if (sarea_priv->nbox > R128_NR_SAREA_CLIPRECTS)
sarea_priv->nbox = R128_NR_SAREA_CLIPRECTS;

Expand Down Expand Up @@ -1312,6 +1316,8 @@ static int r128_cce_flip(struct drm_device *dev, void *data, struct drm_file *fi

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

RING_SPACE_TEST_WITH_RETURN(dev_priv);

if (!dev_priv->page_flipping)
Expand All @@ -1331,6 +1337,8 @@ static int r128_cce_swap(struct drm_device *dev, void *data, struct drm_file *fi

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

RING_SPACE_TEST_WITH_RETURN(dev_priv);

if (sarea_priv->nbox > R128_NR_SAREA_CLIPRECTS)
Expand All @@ -1354,10 +1362,7 @@ static int r128_cce_vertex(struct drm_device *dev, void *data, struct drm_file *

LOCK_TEST_WITH_RETURN(dev, file_priv);

if (!dev_priv) {
DRM_ERROR("called with no initialization\n");
return -EINVAL;
}
DEV_INIT_TEST_WITH_RETURN(dev_priv);

DRM_DEBUG("pid=%d index=%d count=%d discard=%d\n",
DRM_CURRENTPID, vertex->idx, vertex->count, vertex->discard);
Expand Down Expand Up @@ -1410,10 +1415,7 @@ static int r128_cce_indices(struct drm_device *dev, void *data, struct drm_file

LOCK_TEST_WITH_RETURN(dev, file_priv);

if (!dev_priv) {
DRM_ERROR("called with no initialization\n");
return -EINVAL;
}
DEV_INIT_TEST_WITH_RETURN(dev_priv);

DRM_DEBUG("pid=%d buf=%d s=%d e=%d d=%d\n", DRM_CURRENTPID,
elts->idx, elts->start, elts->end, elts->discard);
Expand Down Expand Up @@ -1476,6 +1478,8 @@ static int r128_cce_blit(struct drm_device *dev, void *data, struct drm_file *fi

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

DRM_DEBUG("pid=%d index=%d\n", DRM_CURRENTPID, blit->idx);

if (blit->idx < 0 || blit->idx >= dma->buf_count) {
Expand All @@ -1501,6 +1505,8 @@ static int r128_cce_depth(struct drm_device *dev, void *data, struct drm_file *f

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

RING_SPACE_TEST_WITH_RETURN(dev_priv);

ret = -EINVAL;
Expand Down Expand Up @@ -1531,6 +1537,8 @@ static int r128_cce_stipple(struct drm_device *dev, void *data, struct drm_file

LOCK_TEST_WITH_RETURN(dev, file_priv);

DEV_INIT_TEST_WITH_RETURN(dev_priv);

if (DRM_COPY_FROM_USER(&mask, stipple->mask, 32 * sizeof(u32)))
return -EFAULT;

Expand All @@ -1555,10 +1563,7 @@ static int r128_cce_indirect(struct drm_device *dev, void *data, struct drm_file

LOCK_TEST_WITH_RETURN(dev, file_priv);

if (!dev_priv) {
DRM_ERROR("called with no initialization\n");
return -EINVAL;
}
DEV_INIT_TEST_WITH_RETURN(dev_priv);

DRM_DEBUG("idx=%d s=%d e=%d d=%d\n",
indirect->idx, indirect->start, indirect->end,
Expand Down Expand Up @@ -1620,10 +1625,7 @@ static int r128_getparam(struct drm_device *dev, void *data, struct drm_file *fi
drm_r128_getparam_t *param = data;
int value;

if (!dev_priv) {
DRM_ERROR("called with no initialization\n");
return -EINVAL;
}
DEV_INIT_TEST_WITH_RETURN(dev_priv);

DRM_DEBUG("pid=%d\n", DRM_CURRENTPID);

Expand Down

0 comments on commit 7dc482d

Please sign in to comment.