Re: [PATCH v4 15/15] dmaengine: dw-edma: Add pcim_iomap_table return checker
From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2021-02-08 20:49:16
Also in:
linux-pci, lkml
[+cc Krzysztof] From reading the subject, I thought you were adding a function to check the return values, i.e., a "checker." But you're really adding "checks" :) On Wed, Feb 03, 2021 at 10:58:06PM +0100, Gustavo Pimentel wrote:
quoted hunk ↗ jump to hunk
Detected by CoverityScan CID 16555 ("Dereference null return") Signed-off-by: Gustavo Pimentel <redacted> --- drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c index 686b4ff..7445033 100644 --- a/drivers/dma/dw-edma/dw-edma-pcie.c +++ b/drivers/dma/dw-edma/dw-edma-pcie.c@@ -238,6 +238,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev, dw->rd_ch_cnt = vsec_data.rd_ch_cnt; dw->rg_region.vaddr = pcim_iomap_table(pdev)[vsec_data.rg.bar]; + if (!dw->rg_region.vaddr) + return -ENOMEM;
This doesn't seem quite right. If pcim_iomap_table() fails, it returns NULL. But then we assign "vaddr = NULL[vsec_data.rg.bar]" which dereferences the NULL pointer even before your test. This "pcim_iomap_table(dev)[n]" pattern is extremely common. There are over 100 calls of pcim_iomap_table(), and $ git grep "pcim_iomap_table(.*)\[.*\]" | wc -l says about 75 of them are of this form, where we dereference the result before testing it.
quoted hunk ↗ jump to hunk
dw->rg_region.vaddr += vsec_data.rg.off; dw->rg_region.paddr = pdev->resource[vsec_data.rg.bar].start; dw->rg_region.paddr += vsec_data.rg.off;@@ -250,12 +253,18 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev, struct dw_edma_block *dt_block = &vsec_data.dt_wr[i]; ll_region->vaddr = pcim_iomap_table(pdev)[ll_block->bar]; + if (!ll_region->vaddr) + return -ENOMEM; + ll_region->vaddr += ll_block->off; ll_region->paddr = pdev->resource[ll_block->bar].start; ll_region->paddr += ll_block->off; ll_region->sz = ll_block->sz; dt_region->vaddr = pcim_iomap_table(pdev)[dt_block->bar]; + if (!dt_region->vaddr) + return -ENOMEM; + dt_region->vaddr += dt_block->off; dt_region->paddr = pdev->resource[dt_block->bar].start; dt_region->paddr += dt_block->off;@@ -269,12 +278,18 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev, struct dw_edma_block *dt_block = &vsec_data.dt_rd[i]; ll_region->vaddr = pcim_iomap_table(pdev)[ll_block->bar]; + if (!ll_region->vaddr) + return -ENOMEM; + ll_region->vaddr += ll_block->off; ll_region->paddr = pdev->resource[ll_block->bar].start; ll_region->paddr += ll_block->off; ll_region->sz = ll_block->sz; dt_region->vaddr = pcim_iomap_table(pdev)[dt_block->bar]; + if (!dt_region->vaddr) + return -ENOMEM; + dt_region->vaddr += dt_block->off; dt_region->paddr = pdev->resource[dt_block->bar].start; dt_region->paddr += dt_block->off;-- 2.7.4