CVE-2010-4527: Linux kernel OSS Sound Card Driver Buffer Overflow

This issue was reported by Dan Rosenberg and it affects systems using the OSS sound card driver for Linux. The vulnerable code resides in load_mixer_volumes() routine which is part of the sound/oss/soundcard.c file. Specifically, here is the susceptible code…

/*
 * Table for configurable mixer volume handling
 */
static mixer_vol_table mixer_vols[MAX_MIXER_DEV];
static int num_mixer_volumes;

int *load_mixer_volumes(char *name, int *levels, int present)
{
        int             i, n;

        for (i = 0; i < num_mixer_volumes; i++) {
                if (strcmp(name, mixer_vols[i].name) == 0) {
     ...
        strcpy(mixer_vols[n].name, name);
     ...
}
EXPORT_SYMBOL(load_mixer_volumes);

The initial ‘for’ loop iterates through the ‘mixer_vols[]’ array and uses strcmp(3) to find the requested name. However, there is no check that the provided name is 32 or less Bytes long. This could result in a read out of bounds condition since the name member is statically allocated as we can see at include/linux/soundcard.h header file where that structure is defined.

typedef struct mixer_vol_table {
  int num;      /* Index to volume table */
  char name[32];
  int levels[32];
} mixer_vol_table;

Of course, the subsequent call to strcpy(3) lacks of the exact same length checks leading to a possible kernel stack buffer overflow if a user provides a name larger than 32 Bytes long. The fix for the first issue was to use the strncmp(3) routine that performs the same task having a maximum acceptable size.

 	for (i = 0; i < num_mixer_volumes; i++) {
-		if (strcmp(name, mixer_vols[i].name) == 0) {
+		if (strncmp(name, mixer_vols[i].name, 32) == 0) {
 			if (present)

And similarly, strcpy(3) was replaced with the strncpy(3) one…

 	n = num_mixer_volumes++;
 
-	strcpy(mixer_vols[n].name, name);
+	strncpy(mixer_vols[n].name, name, 32);
 
 	if (present)

As Dan Rosenberg pointed out, the vulnerable routine is reachable through the following IOCTL command…

static long sound_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
        int len = 0, dtype;
     ...
        switch (dev & 0x0f) {
     ...
                else if (cmd == SOUND_MIXER_SETLEVELS)
                        ret = set_mixer_levels(p);
     ...
        unlock_kernel();
        return ret;
}

Where set_mixer_levels() is nothing more than a wrapper around load_mixer_volumes() as we can see here:

static int set_mixer_levels(void __user * arg)
{
        /* mixer_vol_table is 174 bytes, so IMHO no reason to not allocate it on the stack */
        mixer_vol_table buf;   

        if (__copy_from_user(&buf, arg, sizeof(buf)))
                return -EFAULT;
        load_mixer_volumes(buf.name, buf.levels, 0);
        if (__copy_to_user(arg, &buf, sizeof(buf)))
                return -EFAULT;
        return 0;
}