xorl %eax, %eax

CVE-2011-2700: Linux kernel si4713-i2c Buffer Overflow

leave a comment »

The following issue was reported by Mauro Carvalho Chehab of Red Hat. The buggy code is part of Silicon Labs Si4713 FM Radio Transmitter I2C driver which according to Eugene Teo is only used by N900.

The susceptible code is available in drivers/media/radio/si4713-i2c.c file and here is the exact code snippet…

/*
 * Video4Linux Subdev Interface
 */
/* si4713_s_ext_ctrls - set extended controls value */
static int si4713_s_ext_ctrls(struct v4l2_subdev *sd,
                                struct v4l2_ext_controls *ctrls)
{
   ...
                switch ((ctrls->controls + i)->id) {
                case V4L2_CID_RDS_TX_PS_NAME:
                case V4L2_CID_RDS_TX_RADIO_TEXT:
                        err = si4713_write_econtrol_string(sdev,
                                                        ctrls->controls + i);
                        break;
   ...
        return 0;
}
   ...
static const struct v4l2_subdev_core_ops si4713_subdev_core_ops = {
        .queryctrl      = si4713_queryctrl,
        .g_ext_ctrls    = si4713_g_ext_ctrls,
        .s_ext_ctrls    = si4713_s_ext_ctrls,
        .g_ctrl         = si4713_g_ctrl,
        .s_ctrl         = si4713_s_ctrl,
        .ioctl          = si4713_ioctl,
}

So, using the si4713_s_ext_ctrls() operation and the ‘V4L2_CID_RDS_TX_PS_NAM’ or ‘V4L2_CID_RDS_TX_RADIO_TEX’ control IDs, a user could reach si4713_write_econtrol_string() function which is shown below.

/* write string property */
static int si4713_write_econtrol_string(struct si4713_device *sdev,
                                struct v4l2_ext_control *control)
{
        struct v4l2_queryctrl vqc;
        int len;
        s32 rval = 0;

        vqc.id = control->id;
        rval = si4713_queryctrl(&sdev->sd, &vqc);
        if (rval < 0)
                goto exit;

        switch (control->id) {
        case V4L2_CID_RDS_TX_PS_NAME: {
                char ps_name[MAX_RDS_PS_NAME + 1];

                len = control->size - 1;
                if (len > MAX_RDS_PS_NAME) {
                        rval = -ERANGE;
                        goto exit;
                }
                rval = copy_from_user(ps_name, control->string, len);
   ...
        case V4L2_CID_RDS_TX_RADIO_TEXT: {
                char radio_text[MAX_RDS_RADIO_TEXT + 1];

                len = control->size - 1;
                if (len > MAX_RDS_RADIO_TEXT) {
                        rval = -ERANGE;
                        goto exit;
                }
                rval = copy_from_user(radio_text, control->string, len);
   ...
exit:
        return rval;
}

As you can see, in both cases the ‘len’ singed integer is initialized by the passed ‘control->size’ and it is only checked against the maximum allowed value. Altough copy_from_user() on x86/x86_64 would prevent any negative values from being used, other architectures might be vulnerable.
Of course, the fix was to add the missing check…

                len = control->size - 1;
-               if (len > MAX_RDS_PS_NAME) {
+               if (len < 0 || len > MAX_RDS_PS_NAME) {
                        rval = -ERANGE;

And equivalently in the second case…

                len = control->size - 1;
-               if (len > MAX_RDS_RADIO_TEXT) {
+               if (len < 0 || len > MAX_RDS_RADIO_TEXT) {
                        rval = -ERANGE;

Written by xorl

July 24, 2011 at 16:12

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