CVE-2011-1494: Linux kernel /dev/mpt2ctl IOCTL Heap Overflows

These are two neat vulnerabilities in the MPT Fusion support of the Linux kernel were reported by Dan Rosenberg as we can see in his email to linux-kernel mailing list on 05 April 2011. The first issue resides in drivers/scsi/mpt2sas/mpt2sas_ctl.c and more specifically in the following routine.

/**
 * _ctl_do_mpt_command - main handler for MPT2COMMAND opcode
 * @ioc: per adapter object
 * @karg - (struct mpt2_ioctl_command)
 * @mf - pointer to mf in user space
 * @state - NON_BLOCKING or BLOCKING
 */
static long
_ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
    struct mpt2_ioctl_command karg, void __user *mf, enum block_state state)
{
        MPI2RequestHeader_t *mpi_request = NULL, *request;
        MPI2DefaultReply_t *mpi_reply;
  ...
        mpi_request = kzalloc(ioc->request_sz, GFP_KERNEL);
        if (!mpi_request) {
                printk(MPT2SAS_ERR_FMT "%s: failed obtaining a memory for "
                    "mpi_request\n", ioc->name, __func__);
                ret = -ENOMEM;
                goto out;
        }

        /* copy in request message frame from user */
        if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
                printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__, __LINE__,
                    __func__);
                ret = -EFAULT;
                goto out;
        }
  ...
        mutex_unlock(&ioc->ctl_cmds.mutex);
        return ret;
}

The ‘mpi_request’ allocates exactly ‘ioc->request_sz’ using kzalloc() but the mutliplication by 4 in the copy_from_user() call can result in an integer overflow which in turn will lead to a heap memory corruption. This was fixed by applying the patch you see below.

 	}
 
+	/* Check for overflow and wraparound */
+	if (karg.data_sge_offset * 4 > ioc->request_sz ||
+	    karg.data_sge_offset > (UINT_MAX / 4)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/* copy in request message frame from user */

Which checks for integer overflows before proceeding to copying the data.

The second one is part of the next function.

/**
 * _ctl_diag_release - request to send Diag Release Message to firmware
 * @arg - user space buffer containing ioctl content
 * @state - NON_BLOCKING or BLOCKING
 *
 * This allows ownership of the specified buffer to returned to the driver,
 * allowing an application to read the buffer without fear that firmware is
 * overwritting information in the buffer.
 */
static long
_ctl_diag_release(void __user *arg, enum block_state state)
{
        struct mpt2_diag_release karg;
        struct MPT2SAS_ADAPTER *ioc;
        void *request_data;
        int rc;
        u8 buffer_type;
        u8 issue_reset = 0;

        if (copy_from_user(&karg, arg, sizeof(karg))) 
  ...
        if ((karg.starting_offset % 4) || (karg.bytes_to_read % 4)) {
                printk(MPT2SAS_ERR_FMT "%s: either the starting_offset "
                    "or bytes_to_read are not 4 byte aligned\n", ioc->name,
                    __func__);
                return -EINVAL;
        }

        diag_data = (void *)(request_data + karg.starting_offset);
  ...
out:

        ioc->ctl_cmds.status = MPT2_CMD_NOT_USED;
        mutex_unlock(&ioc->ctl_cmds.mutex);
        return rc;
}

Here the addition of ‘request_data’ with ‘karg.starting_offset’ could result in pointing to heap data beyond the bounds of ‘request_data’ since the ‘karg’ structure is completely user controlled. To fix this, apart from updating the data type of some variables.

 	u8 buffer_type;
-	unsigned long timeleft;
+	unsigned long timeleft, request_size, copy_size;
 	u16 smid;

The ‘request_size’ was added to include the bound address of the buffer.

 	}
 
+	request_size = ioc->diag_buffer_sz[buffer_type];
+
 	if ((karg.starting_offset % 4) || (karg.bytes_to_read % 4)) {

And a new check was added to ensure that the user controlled value is always within the boundaries.

 	}
 
+	if (karg.starting_offset > request_size)
+		return -EINVAL;
+
 	diag_data = (void *)(request_data + karg.starting_offset);