xorl %eax, %eax

CVE-2011-1746: Linux kernel AGPgart Multiple Integer Overflows

leave a comment »

On April 2011, Vasiliy Kulikov reported this vulnerability. The bug is part of the drivers/char/agp/generic.c file which contains the code for AGPgart device driver. Here is the susceptible routine…

/**
 *      agp_allocate_memory  -  allocate a group of pages of a certain type.
 *
 *      @page_count:    size_t argument of the number of pages
 *      @type:  u32 argument of the type of memory to be allocated.
 *
 *      Every agp bridge device will allow you to allocate AGP_NORMAL_MEMORY which
 *      maps to physical ram.  Any other type is device dependent.
 *
 *      It returns NULL whenever memory is unavailable.
 */
struct agp_memory *agp_allocate_memory(struct agp_bridge_data *bridge,
                                        size_t page_count, u32 type)
{
        int scratch_pages;
        struct agp_memory *new;
        size_t i;

        if (!bridge)
                return NULL;

        if ((atomic_read(&bridge->current_memory_agp) + page_count) > bridge->max_memory_agp)
                return NULL;

        if (type >= AGP_USER_TYPES) {
                new = agp_generic_alloc_user(page_count, type);
                if (new)
                        new->bridge = bridge;
                return new;
        }
  ...
        return new;
}
EXPORT_SYMBOL(agp_allocate_memory);

As Vasiliy Kulikov noticed, the ‘page_count’ value is copied from user-space and the above routine will only check that it does not exceed the ‘bridge->max_memory_agp’ limit. However, the addition could result in an integer overflow and thus bypass this check and result in kernel memory corruption due to a small memory allocation.
To fix this, the following patch was applied.

 	size_t i;
+	int cur_memory;
 
 	if (!bridge)
 		return NULL;
 
-	if ((atomic_read(&bridge->current_memory_agp) + page_count) > bridge->max_memory_agp)
+	cur_memory = atomic_read(&bridge->current_memory_agp);
+	if ((cur_memory + page_count > bridge->max_memory_agp) ||
+	    (cur_memory + page_count < page_count))
 		return NULL;

Which also checks for the integer overflow case by comparing the memory size to be allocated with the ‘page_count’ value.

The second issue addressed in this patch is a bug in agp_create_user_memory() you see below.

static struct agp_memory *agp_create_user_memory(unsigned long num_agp_pages)
{
        struct agp_memory *new;
        unsigned long alloc_size = num_agp_pages*sizeof(struct page *);

        new = kzalloc(sizeof(struct agp_memory), GFP_KERNEL);
        if (new == NULL)
                return NULL;

        new->key = agp_get_key();
  ...
        new->num_scratch_pages = 0;
        return new;
}

Once again, the ‘allog_size’ could result to an integer overflow due to the multiplication that could allocate a smaller than the required heap memory area using kzalloc() and eventually lead to heap memory corruption. This was fixed by adding the missing checks…

 	unsigned long alloc_size = num_agp_pages*sizeof(struct page *);
 
+	if (INT_MAX/sizeof(struct page *) < num_agp_pages)
+		return NULL;
+
 	new = kzalloc(sizeof(struct agp_memory), GFP_KERNEL);

Written by xorl

May 28, 2011 at 17:43

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