* Re: [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller
@ 2018-10-04 8:28 ` Colin Ian King
0 siblings, 0 replies; 6+ messages in thread
From: Colin Ian King @ 2018-10-04 8:28 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, linux-scsi
Cc: linux-kernel, kernel-janitors
Hi,
Static analysis from CoverityScan (CID#114178 "Operands don't affect
result") detected an issue in drivers/scsi/pmcraid.c, function
pmcraid_init_res_table with the following check:
if (pinstance->cfg_table->flags & MICROCODE_UPDATE_REQUIRED)
pmcraid_err("IOA requires microcode download\n");
pinstance->cfg_table->flags is a u8, MICROCODE_UPDATE_REQUIRED is 1 <<
31, so the & operation always results in false and the error message is
never displayed. From my understanding, flags should be a u8, so there
is something wrong here with the check. Any ideas?
Colin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller
@ 2018-10-04 8:28 ` Colin Ian King
0 siblings, 0 replies; 6+ messages in thread
From: Colin Ian King @ 2018-10-04 8:28 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, linux-scsi
Cc: linux-kernel, kernel-janitors
Hi,
Static analysis from CoverityScan (CID#114178 "Operands don't affect
result") detected an issue in drivers/scsi/pmcraid.c, function
pmcraid_init_res_table with the following check:
if (pinstance->cfg_table->flags & MICROCODE_UPDATE_REQUIRED)
pmcraid_err("IOA requires microcode download\n");
pinstance->cfg_table->flags is a u8, MICROCODE_UPDATE_REQUIRED is 1 <<
31, so the & operation always results in false and the error message is
never displayed. From my understanding, flags should be a u8, so there
is something wrong here with the check. Any ideas?
Colin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller
2018-10-04 8:28 ` Colin Ian King
@ 2018-10-04 11:48 ` John Garry
-1 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2018-10-04 11:48 UTC (permalink / raw)
To: Colin Ian King, James E.J. Bottomley, Martin K. Petersen, linux-scsi
Cc: linux-kernel, kernel-janitors
On 04/10/2018 09:28, Colin Ian King wrote:
> Hi,
>
> Static analysis from CoverityScan (CID#114178 "Operands don't affect
> result") detected an issue in drivers/scsi/pmcraid.c, function
> pmcraid_init_res_table with the following check:
>
> if (pinstance->cfg_table->flags & MICROCODE_UPDATE_REQUIRED)
> pmcraid_err("IOA requires microcode download\n");
>
>
> pinstance->cfg_table->flags is a u8, MICROCODE_UPDATE_REQUIRED is 1 <<
> 31, so the & operation always results in false and the error message is
> never displayed. From my understanding, flags should be a u8, so there
> is something wrong here with the check. Any ideas?
>
> Colin
>
>
Maybe MICROCODE_UPDATE_REQUIRED definition is wrong and should be:
/* config_table.flags value */
-#define MICROCODE_UPDATE_REQUIRED PMC_BIT32(0)
+#define MICROCODE_UPDATE_REQUIRED PMC_BIT8(0)
John
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller
@ 2018-10-04 11:48 ` John Garry
0 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2018-10-04 11:48 UTC (permalink / raw)
To: Colin Ian King, James E.J. Bottomley, Martin K. Petersen, linux-scsi
Cc: linux-kernel, kernel-janitors
On 04/10/2018 09:28, Colin Ian King wrote:
> Hi,
>
> Static analysis from CoverityScan (CID#114178 "Operands don't affect
> result") detected an issue in drivers/scsi/pmcraid.c, function
> pmcraid_init_res_table with the following check:
>
> if (pinstance->cfg_table->flags & MICROCODE_UPDATE_REQUIRED)
> pmcraid_err("IOA requires microcode download\n");
>
>
> pinstance->cfg_table->flags is a u8, MICROCODE_UPDATE_REQUIRED is 1 <<
> 31, so the & operation always results in false and the error message is
> never displayed. From my understanding, flags should be a u8, so there
> is something wrong here with the check. Any ideas?
>
> Colin
>
>
Maybe MICROCODE_UPDATE_REQUIRED definition is wrong and should be:
/* config_table.flags value */
-#define MICROCODE_UPDATE_REQUIRED PMC_BIT32(0)
+#define MICROCODE_UPDATE_REQUIRED PMC_BIT8(0)
John
^ permalink raw reply [flat|nested] 6+ messages in thread
* re: [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller
@ 2016-01-29 10:39 Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2016-01-29 10:39 UTC (permalink / raw)
To: anil_ravindranath; +Cc: linux-scsi
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* re: [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller
@ 2015-01-06 9:53 Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-01-06 9:53 UTC (permalink / raw)
To: anil_ravindranath; +Cc: linux-scsi
(Sorry, this code is really old, and normally I wouldn't report it but
I'm making a tutorial).
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:3860 pmcraid_ioctl_passthrough()
error: potential NULL dereference 'cmd->scsi_cmd'.
drivers/scsi/pmcraid.c
3764 cmd->scsi_cmd = NULL;
^^^^^^^^^^^^^^^^^^^^
We set this to NULL here.
3765 ioarcb = &(cmd->ioa_cb->ioarcb);
3766
3767 /* Copy the user-provided IOARCB stuff field by field */
3768 ioarcb->resource_handle = buffer->ioarcb.resource_handle;
3769 ioarcb->data_transfer_length = buffer->ioarcb.data_transfer_length;
3770 ioarcb->cmd_timeout = buffer->ioarcb.cmd_timeout;
3771 ioarcb->request_type = buffer->ioarcb.request_type;
3772 ioarcb->request_flags0 = buffer->ioarcb.request_flags0;
3773 ioarcb->request_flags1 = buffer->ioarcb.request_flags1;
3774 memcpy(ioarcb->cdb, buffer->ioarcb.cdb, PMCRAID_MAX_CDB_LEN);
3775
3776 if (buffer->ioarcb.add_cmd_param_length) {
3777 ioarcb->add_cmd_param_length =
3778 buffer->ioarcb.add_cmd_param_length;
3779 ioarcb->add_cmd_param_offset =
3780 buffer->ioarcb.add_cmd_param_offset;
3781 memcpy(ioarcb->add_data.u.add_cmd_params,
3782 buffer->ioarcb.add_data.u.add_cmd_params,
3783 buffer->ioarcb.add_cmd_param_length);
3784 }
3785
3786 /* set hrrq number where the IOA should respond to. Note that all cmds
3787 * generated internally uses hrrq_id 0, exception to this is the cmd
3788 * block of scsi_cmd which is re-used (e.g. cancel/abort), which uses
3789 * hrrq_id assigned here in queuecommand
3790 */
3791 ioarcb->hrrq_id = atomic_add_return(1, &(pinstance->last_message_id)) %
3792 pinstance->num_hrrq;
3793
[ snip ]
3857 cmd->ioa_cb->ioarcb.cdb[0]);
3858
3859 spin_lock_irqsave(pinstance->host->host_lock, lock_flags);
3860 cancel_cmd = pmcraid_abort_cmd(cmd);
^^^^^^^^^^^^^^^^^^^^^^
This will Oops because cmd->scsi_cmd is NULL.
3861 spin_unlock_irqrestore(pinstance->host->host_lock, lock_flags);
3862
3863 if (cancel_cmd) {
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-04 11:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 8:28 [SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller Colin Ian King
2018-10-04 8:28 ` Colin Ian King
2018-10-04 11:48 ` John Garry
2018-10-04 11:48 ` John Garry
-- strict thread matches above, loose matches on Subject: below --
2016-01-29 10:39 Dan Carpenter
2015-01-06 9:53 Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.