CVE-2011-1477: Linux kernel Yamaha YM3812 and OPL-3 Arbitrary Write

This is an interesting vulnerability reported by Dan Rosenberg and the buggy code is available at sound/oss/opl3.c which is the Linux kernel low level device driver implementation for Yamaha YM3812 and OPL-3 chips.

static struct synth_operations opl3_operations =
{
        .owner          = THIS_MODULE,
        .id             = "OPL",
        .info           = NULL,
        .midi_dev       = 0,
        .synth_type     = SYNTH_TYPE_FM,
        .synth_subtype  = FM_TYPE_ADLIB,
        .open           = opl3_open,
        .close          = opl3_close,
        .ioctl          = opl3_ioctl,
        .kill_note      = opl3_kill_note,
        .start_note     = opl3_start_note,
        .set_instr      = opl3_set_instr,
        .reset          = opl3_reset,
        .hw_control     = opl3_hw_control,
        .load_patch     = opl3_load_patch,
        .aftertouch     = opl3_aftertouch,
        .controller     = opl3_controller,
        .panning        = opl3_panning,
        .volume_method  = opl3_volume_method,
        .bender         = opl3_bender,
        .alloc_voice    = opl3_alloc_voice,
        .setup_voice    = opl3_setup_voice
};

Among the numerous operations accessible via the ‘/dev/sequencer’ device we can see the ‘panning’ operation which is handled by the opl3_panning() routine located in the same source code file.

static void opl3_panning(int dev, int voice, int value)
{
        devc->voc[voice].panning = value;
}

At first it seems to be fine assuming that the ‘devc->voc[]’ array is dynamically allocated based on the index value. However, a closer look reveals that this is statically allocated…

#define MAX_VOICE       18
  ...
typedef struct opl_devinfo
{
        int             base;
        int             left_io, right_io;
        int             nr_voice;
        int             lv_map[MAX_VOICE];

        struct voice_info voc[MAX_VOICE];
        struct voice_alloc_info *v_alloc;
        struct channel_info *chn_info;

        struct sbi_instrument i_map[SBFM_MAXINSTR];
        struct sbi_instrument *act_i[MAX_VOICE];

        struct synth_info fm_info;

        int             busy;
        int             model;
        unsigned char   cmask;

        int             is_opl4;
} opl_devinfo;

static struct opl_devinfo *devc = NULL;
  ...
static int opl3_detect(int ioaddr)
{
  ...
        if (devc != NULL)
        {
                printk(KERN_ERR "opl3: Only one OPL3 supported.\n");
                return 0;
        }

        devc = kzalloc(sizeof(*devc), GFP_KERNEL);
  ...
}

Consequently, this means that any index value greater or less than ‘MAX_VOICE’ will result in an arbitrary kernel heap write operation. This was fixed by adding the missing check.

 static void opl3_panning(int dev, int voice, int value)
 {
+
+	if (voice < 0 || voice >= devc->nr_voice)
+		return;
+
 	devc->voc[voice].panning = value;
 }

A similar vulnerability was also present in opl3_alloc_voice() which is the handler routine for the ‘alloc_voice’ operation. Here is this function.

static void opl3_setup_voice(int dev, int voice, int chn)
{
        struct channel_info *info =
        &synth_devs[dev]->chn_info[chn];

        opl3_set_instr(dev, voice, info->pgm_num);

        devc->voc[voice].bender = 0;
        devc->voc[voice].bender_range = info->bender_range;
        devc->voc[voice].volume = info->controllers[CTL_MAIN_VOLUME];
        devc->voc[voice].panning = (info->controllers[CTL_PAN] * 2) - 128;
}

Here both the ‘voice’ and ‘chn’ are not checked prior to their use leading to identical vulnerabilities. As you might have been expecting, the patch to fix this issue was:

 static void opl3_setup_voice(int dev, int voice, int chn)
 {
-	struct channel_info *info =
-	&synth_devs[dev]->chn_info[chn];
+	struct channel_info *info;
+
+	if (voice < 0 || voice >= devc->nr_voice)
+		return;
+
+	if (chn < 0 || chn > 15)
+		return;
+
+	info = &synth_devs[dev]->chn_info[chn];
 
 	opl3_set_instr(dev, voice, info->pgm_num);