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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
