xorl %eax, %eax

CVE-2011-1745: Linux kernel agp_ioctl() Arbitrary Write

leave a comment »

This issue was reported by Vasiliy Kulikov and we can find the vulnerable code at drivers/char/agp/generic.c where the AGPgart (Graphics Address Remapping Table) device driver resides. If we move to the IOCTL handler at drivers/char/agp/frontend.c we will notice the following.

static long agp_ioctl(struct file *file,
                     unsigned int cmd, unsigned long arg)
{
        struct agp_file_private *curr_priv = file->private_data;
        int ret_val = -ENOTTY;
   ...
        switch (cmd) {
   ...
        case AGPIOC_BIND:
                ret_val = agpioc_bind_wrap(curr_priv, (void __user *) arg);
                break;

        case AGPIOC_UNBIND:
                ret_val = agpioc_unbind_wrap(curr_priv, (void __user *) arg);
                break;
   ...
ioctl_out:
        DBG("ioctl returns %d\n", ret_val);
        mutex_unlock(&(agp_fe.agp_mutex));
        return ret_val;
}

As it was already noted in the official bug report, this would almost certainly require sufficient privileges (meaning group access of “video”). So, in case of AGPIOC_BIND the execution is transfered to agpioc_bind_wrap() that includes the following.

static int agpioc_bind_wrap(struct agp_file_private *priv, void __user *arg)
{
        struct agp_bind bind_info;
        struct agp_memory *memory;

        DBG("");
        if (copy_from_user(&bind_info, arg, sizeof(struct agp_bind)))
                return -EFAULT;

        memory = agp_find_mem_by_key(bind_info.key);

        if (memory == NULL)
                return -EINVAL;

        return agp_bind_memory(memory, bind_info.pg_start);
}

Which simply copies the user supplied structure from user-land to kernel space and invokes agp_bind_memory() in order to copy the information structure. Among others the latter routine will invoke a callback function as you can see here:

int agp_bind_memory(struct agp_memory *curr, off_t pg_start)
{
        int ret_val;

        if (curr == NULL)
                return -EINVAL;
  ...
        ret_val = curr->bridge->driver->insert_memory(curr, pg_start, curr->type);
  ...
        return 0;
}
EXPORT_SYMBOL(agp_bind_memory);

This callback translates to

int agp_generic_insert_memory(struct agp_memory * mem, off_t pg_start, int type)
{
        int num_entries;
        size_t i;
        off_t j;
        void *temp;
        struct agp_bridge_data *bridge;
        int mask_type;
 ...
        /* AK: could wrap */
        if ((pg_start + mem->page_count) > num_entries)
                return -EINVAL;

        j = pg_start;

        while (j < (pg_start + mem->page_count)) {
                if (!PGE_EMPTY(bridge, readl(bridge->gatt_table+j)))
                        return -EBUSY;
                j++;
        }
  ...
        readl(bridge->gatt_table+j-1);  /* PCI Posting. */

        bridge->driver->tlb_flush(mem);
        return 0;
}
EXPORT_SYMBOL(agp_generic_insert_memory);

As it is already noted in the comment, the memory calculation could wrap and consequently lead to kernel memory corruption. In addition, on the second code path of AGPIOC_UNBIND IOCTL command, the equivalent agp_generic_remove_memory() does not check this value at all.

So, the fix was to patch agp_generic_insert_memory() in order to check the pointer’s bounds…

-	/* AK: could wrap */
-	if ((pg_start + mem->page_count) > num_entries)
+	if (((pg_start + mem->page_count) > num_entries) ||
+	    ((pg_start + mem->page_count) < pg_start))
 		return -EINVAL;

And a new integer was added in agp_generic_remove_memory()

 	struct agp_bridge_data *bridge;
-	int mask_type;
+	int mask_type, num_entries;
 
 	bridge = mem->bridge;

That is used in an identical check you see below.

+	num_entries = agp_num_entries();
+	if (((pg_start + mem->page_count) > num_entries) ||
+	    ((pg_start + mem->page_count) < pg_start))
+		return -EINVAL;
+
 	mask_type = bridge->driver->agp_type_to_mask_type(bridge, type);

Written by xorl

May 28, 2011 at 17:40

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