pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller

Hello Anil Ravindranath,

The patch 89a368104150: "[SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller" from Aug 25, 2009, leads to the following static checker warning:

drivers/scsi/pmcraid.c:3376 pmcraid_copy_sglist()
error: overflow detected. __copy_from_user() 'kaddr' is 4096 bytes. limit = '0-u32max'

drivers/scsi/pmcraid.c
3331 static int pmcraid_copy_sglist(

  3332          struct pmcraid_sglist *sglist,
  3333          unsigned long buffer,
  3334          u32 len,
  3335          int direction
  3336  )
  3337  {
  3338          struct scatterlist *scatterlist;
  3339          void *kaddr;
  3340          int bsize_elem;
  3341          int i;
  3342          int rc = 0;
  3343  
  3344          /* Determine the actual number of bytes per element */
  3345          bsize_elem = PAGE_SIZE * (1 << sglist->order);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This sets bsize_elem to PAGE_SIZE or higher.

3346
3347 scatterlist = sglist->scatterlist;
3348
3349 for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
3350 struct page *page = sg_page(&scatterlist[i]);
3351
3352 kaddr = kmap(page);
3353 if (direction == DMA_TO_DEVICE)
3354 rc = __copy_from_user(kaddr,
3355 (void *)buffer,
3356 bsize_elem);
3357 else
3358 rc = __copy_to_user((void *)buffer, kaddr, bsize_elem);
3359
3360 kunmap(page);
3361
3362 if (rc) {
3363 pmcraid_err("failed to copy user data into sg list\n");
3364 return -EFAULT;
3365 }
3366
3367 scatterlist[i].length = bsize_elem;
3368 }
3369
3370 if (len % bsize_elem) {
3371 struct page *page = sg_page(&scatterlist[i]);
3372

  3373                  kaddr = kmap(page);
                        ^^^^^^^^^^^^^^^^^^

This maps a single page. Apparently, on x86_64 it's a no-op? Likely this code was not tested on a HIGHMEM system (x86_32 with more than 1G of RAM).

3374
3375 if (direction == DMA_TO_DEVICE)
3376 rc = __copy_from_user(kaddr,

                                                      ^^^^^
  3377                                                (void *)buffer,
  3378                                                len % bsize_elem);
                                                      ^^^^^^^^^^^^^^^^^

We're copying more than PAGE_SIZE potentially.

Anyway, something odd is going on here, and I don't know what to do about it.

  3379                  else
  3380                          rc = __copy_to_user((void *)buffer,
  3381                                              kaddr,
  3382                                              len % bsize_elem);
  3383  
  3384                  kunmap(page);
  3385  
  3386                  scatterlist[i].length = len % bsize_elem;
  3387          }

regards,
dan carpenter