xorl %eax, %eax

CVE-2009-3620: Linux kernel r128 Multiple Vulnerabilities

leave a comment »

These vulnerabilities were reported by Ben Hutchings. All bugs are specific on ATI Rage 128 device driver located under drivers/gpu/drm/r128/ in the Linux kernel source code tree. The first bug appears in the CCE initialization routine, located at drivers/gpu/drm/r128/r128_cce.c.

static int r128_do_init_cce(struct drm_device * dev, drm_r128_init_t * init)
        drm_r128_private_t *dev_priv;


        dev_priv = kzalloc(sizeof(drm_r128_private_t), GFP_KERNEL);
        if (dev_priv == NULL)
                return -ENOMEM;

        dev_priv->is_pci = init->is_pci;
        return 0;

CCE state described by ‘dev_priv’ might be already initialized when calling r128_do_init_cce() and the re-allocation using kzalloc() could result in resource leak or even a crash. This was fixed by adding a check for non-NULL values of ‘dev_priv’ like this:

+       if (dev->dev_private) {
+               DRM_DEBUG("called when already initialized\n");
+               return -EINVAL;
+       }
        dev_priv = kzalloc(sizeof(drm_r128_private_t), GFP_KERNEL);

Now, the rest of the vulnerabilities were a series of possible NULL pointer dereferences in case of an uninitialized CCE state in almost all the IOCTL handling routines. For example, r128_cce_start() contains this code:

int r128_cce_start(struct drm_device *dev, void *data, struct drm_file *file_priv)
        drm_r128_private_t *dev_priv = dev->dev_private;

If ‘dev_priv’ was uninitialized and thus, NULL, the subsequent call to r128_do_cce_start() passing that NULL pointer could result in some exploitable NULL pointer dereference case. Similar bugs were also present in: r128_cce_stop(), r128_cce_reset(), r128_cce_idle(), r128_engine_reset(), r128_cce_flip(),r128_cce_swap(), r128_cce_vertex(), r128_cce_indices(), r128_cce_blit(), r128_cce_depth(), r128_cce_stipple(), r128_cce_indirect() and r128_getparam() routines. To fix them, a new C macro was added in drivers/gpu/drm/r128/r128_drv.h

+#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 )                

which is a common check for NULL pointer in the given argument, and also the initialization in r128_cce_dispatch_stipple() was updated to assign the value after the NULL check like this:

 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;
        LOCK_TEST_WITH_RETURN(dev, file_priv);
+       DEV_INIT_TEST_WITH_RETURN(dev_priv);
+       sarea_priv = dev_priv->sarea_priv;
        if (sarea_priv->nbox > R128_NR_SAREA_CLIPRECTS)

The rest susceptible functions were also patched to include the DEV_INIT_TEST_WITH_RETURN() before using ‘dev_priv’ pointer.

Written by xorl

October 21, 2009 at 20:12

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s