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 <b...@decadent.org.uk>
---
 drivers/gpu/drm/r128/r128_cce.c   |   18 ++++++++++++++----
 drivers/gpu/drm/r128/r128_drv.h   |    8 ++++++++
 drivers/gpu/drm/r128/r128_state.c |   36 +++++++++++++++++++-----------------
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/r128/r128_cce.c b/drivers/gpu/drm/r128/r128_cce.c
index c75fd35..ebf9f63 100644
--- a/drivers/gpu/drm/r128/r128_cce.c
+++ b/drivers/gpu/drm/r128/r128_cce.c
@@ -353,6 +353,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;
@@ -649,6 +654,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;
@@ -671,6 +678,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.
         */
@@ -708,10 +717,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);
 
@@ -728,6 +734,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);
        }
@@ -741,6 +749,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);
 }
 
diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
index 797a26c..3c60829 100644
--- a/drivers/gpu/drm/r128/r128_drv.h
+++ b/drivers/gpu/drm/r128/r128_drv.h
@@ -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;          \
diff --git a/drivers/gpu/drm/r128/r128_state.c 
b/drivers/gpu/drm/r128/r128_state.c
index 026a48c..af2665c 100644
--- a/drivers/gpu/drm/r128/r128_state.c
+++ b/drivers/gpu/drm/r128/r128_state.c
@@ -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;
 
@@ -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)
@@ -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)
@@ -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);
@@ -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);
@@ -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) {
@@ -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;
@@ -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;
 
@@ -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,
@@ -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);
 
-- 
1.6.3.3

Attachment: signature.asc
Description: This is a digitally signed message part

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to