xorl %eax, %eax

CVE-2010-3442: Linux kernel snd_ctl_new() Integer Overflow

leave a comment »

This issue was reported by Dan Rosenberg of Virtual Security Research. The vulnerable code affects Linux kernel prior to 2.6.36-rc5-next-20100929 release and it is available at sound/core/control.c. Specifically, here is the buggy function…

/**
 * snd_ctl_new - create a control instance from the template
 * @control: the control template
 * @access: the default control access
 *
 * Allocates a new struct snd_kcontrol instance and copies the given template 
 * to the new instance. It does not copy volatile data (access).
 *
 * Returns the pointer of the new instance, or NULL on failure.
 */
static struct snd_kcontrol *snd_ctl_new(struct snd_kcontrol *control,
                                        unsigned int access)
{
        struct snd_kcontrol *kctl;
        unsigned int idx;
        
        if (snd_BUG_ON(!control || !control->count))
                return NULL;
        kctl = kzalloc(sizeof(*kctl) + sizeof(struct snd_kcontrol_volatile) * control->count, GFP_KERNEL);
        if (kctl == NULL) {
                snd_printk(KERN_ERR "Cannot allocate control instance\n");
                return NULL;
        }
        *kctl = *control;
        for (idx = 0; idx < kctl->count; idx++)
                kctl->vd[idx].access = access;
        return kctl;
}

It’s quite easy to spot. The calculation inside kzalloc() could easily lead to an integer overflow that will allocate incorrect size of kernel heap memory, possibly leading to heap memory corruption during the ‘for’ loop later in this function. The allocation takes into account the following information:

sizeof(*kctl)                        = Driver's kernel control structure
                                       (not user controlled, but it has known size)
sizeof(struct snd_kcontrol_volatile) = Driver's kernel control structure
                                       (not user controlled, but it has known size)
control->count                       = Driver's kernel control structure
                                       (user controlled unsigned integer)

The above code is reachable easily through the following IOCTL commands.

static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
    ...
        void __user *argp = (void __user *)arg;
        int __user *ip = argp;
        int err;

        ctl = file->private_data;
    ...
        switch (cmd) {
    ...
        case SNDRV_CTL_IOCTL_ELEM_ADD:
                return snd_ctl_elem_add_user(ctl, argp, 0);
        case SNDRV_CTL_IOCTL_ELEM_REPLACE:
                return snd_ctl_elem_add_user(ctl, argp, 1);
    ...
        snd_printdd("unknown ioctl = 0x%x\n", cmd);
        return -ENOTTY;
}

Where both of them lead to:

static int snd_ctl_elem_add_user(struct snd_ctl_file *file,
                                 struct snd_ctl_elem_info __user *_info, int replace)
{
        struct snd_ctl_elem_info info;
        if (copy_from_user(&info, _info, sizeof(info)))
                return -EFAULT;
        return snd_ctl_elem_add(file, &info, replace);
}

Which copies the user’s data using copy_fom_user() kernel API routine and then invokes snd_ctl_elem_add() which can result in calling the buggy snd_ctl_new() as you can see below.

static int snd_ctl_elem_add(struct snd_ctl_file *file,
                            struct snd_ctl_elem_info *info, int replace)
{
        struct snd_card *card = file->card;
        struct snd_kcontrol kctl, *_kctl;
        unsigned int access;
        long private_size;
        struct user_element *ue;
        int idx, err;
    ...
        if (info->count < 1)
                return -EINVAL;
    ...
        _kctl = snd_ctl_new(&kctl, access);
        if (_kctl == NULL) {
                kfree(ue);
                return -ENOMEM;
        }
        _kctl->private_data = ue;
        for (idx = 0; idx < _kctl->count; idx++)
                _kctl->vd[idx].owner = file;
    ...
        return 0;
}

To fix this, the following patch was applied:

        if (snd_BUG_ON(!control || !control->count))
                return NULL;
+
+       if (control->count > MAX_CONTROL_COUNT)
+               return NULL;
+
        kctl = kzalloc(sizeof(*kctl) + sizeof(struct snd_kcontrol_volatile) * control->count, GFP_KERNEL);

Which was placed inside snd_ctl_new() to check for count values greater than ‘MAX_CONTROL_COUNT’ which was in turn was defined at sound/core/control.c like this:

 /* max number of user-defined controls */
 #define MAX_USER_CONTROLS      32
+#define MAX_CONTROL_COUNT      1028
 
 struct snd_kctl_ioctl {

Written by xorl

December 17, 2010 at 22:11

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