xorl %eax, %eax

CVE-2011-1476: Linux kernel OSS copy_from_user() Memory Corruption

leave a comment »

This was also reported by Dan Rosenberg along with the CVE-2011-1477 as part of the OSS bug fixes.
The exact code resides in sound/oss/midi_synth.c file in the function you see below.

int
midi_synth_load_patch(int dev, int format, const char __user *addr,
                      int offs, int count, int pmgr_flag)
{
        int             orig_dev = synth_devs[dev]->midi_dev;

        struct sysex_info sysex;
        int             i;
        unsigned long   left, src_offs, eox_seen = 0;
        int             first_byte = 1;
        int             hdr_size = (unsigned long) &sysex.data[0] - (unsigned long) &sysex;

        leave_sysex(dev);

        if (!prefix_cmd(orig_dev, 0xf0))
                return 0;

        if (format != SYSEX_PATCH)
        {
/*                printk("MIDI Error: Invalid patch format (key) 0x%x\n", format);*/
                  return -EINVAL;
        }
        if (count < hdr_size)
        {
/*              printk("MIDI Error: Patch header too short\n");*/
                return -EINVAL;
        }
        count -= hdr_size;

        /*
         * Copy the header from user space but ignore the first bytes which have
         * been transferred already.
         */

        if(copy_from_user(&((char *) &sysex)[offs], &(addr)[offs], hdr_size - offs))
                return -EFAULT;
 
        if (count < sysex.len)
        {
/*              printk(KERN_WARNING "MIDI Warning: Sysex record too short (%d<%d)\n", count, (int)sysex.len);*/
                sysex.len = count;
        }
        left = sysex.len;
        src_offs = 0;

        for (i = 0; i < left && !signal_pending(current); i++)
        {
                unsigned char   data;

                if (get_user(data,
                    (unsigned char __user *)(addr + hdr_size + i)))
                        return -EFAULT;

                eox_seen = (i > 0 && data & 0x80);      /* End of sysex */

                if (eox_seen && data != 0xf7)
                        data = 0xf7;

                if (i == 0)
                {
                        if (data != 0xf0)
                        {
                                printk(KERN_WARNING "midi_synth: Sysex start missing\n");
                                return -EINVAL;
                        }
                }
                while (!midi_devs[orig_dev]->outputc(orig_dev, (unsigned char) (data & 0xff)) &&
                        !signal_pending(current))
                        schedule();

                if (!first_byte && data & 0x80)
                        return 0;
                first_byte = 0;
        }

        if (!eox_seen)
                midi_outc(orig_dev, 0xf7);
        return 0;
}
EXPORT_SYMBOL(midi_synth_load_patch);

I have included the full routine since it will be easier for the reader to understand the issue. So, before reaching the copy_from_user() call, there is a format check to avoid invalid patch format and a header size one to avoid too short headers. In any case, there is no check of the ‘offs’ value that is used as an offset of both the user and kernel space pointers.
Due to this missing check a user could perform a request on the /dev/sequencer device resulting in an integer overflow of the size limit of copy_from_user() routine resulting in heap memory corruption on some non-x86 based architectures (it does not work on x86 because of the bound checks).

To fix this, this variable was removed from the function.

 int
 midi_synth_load_patch(int dev, int format, const char __user *addr,
-		      int offs, int count, int pmgr_flag)
+		      int count, int pmgr_flag)
 {

And its equivalent definition in sound/oss/midi_synth.h header file.

 int midi_synth_load_patch (int dev, int format, const char __user * addr,
-		 int offs, int count, int pmgr_flag);
+		 int count, int pmgr_flag);
 void midi_synth_panning (int dev, int channel, int pressure);

And the function was updated to prettify the code by adding some comments and removing some unused parts.

 
+	/* Invalid patch format */
 	if (format != SYSEX_PATCH)
-	{
-/*		  printk("MIDI Error: Invalid patch format (key) 0x%x\n", format);*/
 		  return -EINVAL;
-	}
+
+	/* Patch header too short */
 	if (count < hdr_size)
-	{
-/*		printk("MIDI Error: Patch header too short\n");*/
 		return -EINVAL;
-	}
+
 	count -= hdr_size;
 
 	/*
-	 * Copy the header from user space but ignore the first bytes which have
-	 * been transferred already.
+	 * Copy the header from user space
 	 */
 
-	if(copy_from_user(&((char *) &sysex)[offs], &(addr)[offs], hdr_size - offs))
+	if (copy_from_user(&sysex, addr, hdr_size))
 		return -EFAULT;
- 
- 	if (count < sysex.len)
-	{
-/*		printk(KERN_WARNING "MIDI Warning: Sysex record too short (%d<%d)\n", count, (int) sysex.len);*/
+
+	/* Sysex record too short */
+	if ((unsigned)count < (unsigned)sysex.len)
 		sysex.len = count;
-	}
-  	left = sysex.len;
-  	src_offs = 0;
+
+	left = sysex.len;
+	src_offs = 0;
 
 	for (i = 0; i < left && !signal_pending(current); i++)

And you can also see an additional check for Sysex record’s size.

Written by xorl

June 1, 2011 at 18:20

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