Re: [patch] [SCSI] mpt3sas: move dereference under check

2013-03-14 Thread Dan Carpenter
On Thu, Mar 14, 2013 at 06:52:35PM +0530, Reddy, Sreekanth wrote:
 Hi Dan Carpenter,
 
 While analyzing this patch, I have added some debugging prints to
 print the address referenced by the IOC-sense_dma_pool before and
 after pci_pool_free() API and I have observed that the address
 referenced by this pointer is same before and after calling
 pci_pool_free() API.  Please let me know if you have any case
 where you have seen ioc-sense_dma_pool is dereferenced after
 calling pci_pool_free API.
 
 And we are checking this ioc-sense_dma_pool pointer to NULL value
 for safe side even it is not assigned to NULL value anywhere in
 the driver code.

Thanks for looking at this.

I hope you don't mind, but I've added linux-scsi back to the CC.

This is a static checker which is complaining that the code is not
checking for NULL consistently.  If ioc-sense_dma_pool is NULL
then we will crash inside pci_pool_free().  The NULL dereference is
on the line where we do:

mm/dmapool.c
   391  void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma)
   392  {
   393  struct dma_page *page;
   394  unsigned long flags;
   395  unsigned int offset;
   396  
   397  spin_lock_irqsave(pool-lock, flags);
  ^^^
This dereference will cause a crash because we call pci_pool_free()
without testing for NULL.

   398  page = pool_find_page(pool, dma);

In other words:

drivers/scsi/mpt3sas/mpt3sas_base.c
  2481  if (ioc-sense) {
  2482  pci_pool_free(ioc-sense_dma_pool, ioc-sense, 
ioc-sense_dma);
^
We crash on this line before we reach the NULL test on the next
line.

  2483  if (ioc-sense_dma_pool)
  2484  pci_pool_destroy(ioc-sense_dma_pool);

I've looked at the code, and you're right that if (ioc-sense) {
is non-NULL that means -sense_dma_pool is non-NULL.  ioc-sense is
allocated from the DMA pool.

I would like to just remove the test and silence the warning
message.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch] [SCSI] mpt3sas: move dereference under check

2013-03-11 Thread Dan Carpenter
pci_pool_free() dereferences ioc-sense_dma_pool but we check it
for NULL on the following line.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 1836003..06a84ef 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2479,9 +2479,11 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
}
 
if (ioc-sense) {
-   pci_pool_free(ioc-sense_dma_pool, ioc-sense, ioc-sense_dma);
-   if (ioc-sense_dma_pool)
+   if (ioc-sense_dma_pool) {
+   pci_pool_free(ioc-sense_dma_pool, ioc-sense,
+ ioc-sense_dma);
pci_pool_destroy(ioc-sense_dma_pool);
+   }
dexitprintk(ioc, pr_info(MPT3SAS_FMT
sense_pool(0x%p): free\n,
ioc-name, ioc-sense));
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html