[CVE-2011-1494][Maverick][PATCH] mpt2sas: prevent heap overflows and unchecked reads, CVE-2011-1494

[Tetsuo Handa]

Stefan Bader wrote:
> > --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> > +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> > @@ -637,6 +637,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
> >   	data_out_sz = karg.data_out_size;
> >   	data_in_sz = karg.data_in_size;
> >
> > +	/* 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 */
> >   	if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
> >   		printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__, __LINE__,

karg.data_sge_offset is assigned from user-supplied memory.
I don't know whether ioc->request_sz can be set to 0.

But if ioc->request_sz can be set to 0,

683         mpi_request = kzalloc(ioc->request_sz, GFP_KERNEL);

mpi_request is set to ZERO_SIZE_PTR

684         if (!mpi_request) {
685                 printk(MPT2SAS_ERR_FMT "%s: failed obtaining a memory for "
686                     "mpi_request\n", ioc->name, __func__);
687                 ret = -ENOMEM;
688                 goto out;
689         }
690 

and reaches here. If karg.data_sge_offset == 0,

691         /* Check for overflow and wraparound */
692         if (karg.data_sge_offset * 4 > ioc->request_sz ||
693             karg.data_sge_offset > (UINT_MAX / 4)) {
694                 ret = -EINVAL;
695                 goto out;
696         }
697 

reaches here.

copy_from_user() copies no data, but

698         /* copy in request message frame from user */
699         if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
700                 printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__, __LINE__,
701                     __func__);
702                 ret = -EFAULT;
703                 goto out;
704         }
705 

accessing ZERO_SIZE_PTR->Function is an oops.

706         if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) {

Many functions are passing a variable to (e.g.) kmalloc() with an
assumption that the value of that variable is never 0.

Although this was once NACKed at http://lkml.org/lkml/2010/2/26/30 , don't we
want memory allocator functions that return NULL if requested size is 0?


At line 706, mpi_request->Function is always 0 if karg.data_sge_offset == 0 and
ioc->request_sz != 0 ... Maybe we need karg.data_sge_offset != 0 check.