* [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest
@ 2019-04-10 9:34 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-04-10 9:34 UTC (permalink / raw)
To: jean-philippe.brucker-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Robin Murphy
Hello Jean-Philippe Brucker,
This is a semi-automatic email about new static checker warnings.
The patch fac83d29d954: "iommu/io-pgtable-arm: Fix pgtable allocation
in selftest" from Jun 18, 2018, leads to the following Smatch
complaint:
drivers/iommu/io-pgtable-arm.c:246 __arm_lpae_alloc_pages()
error: we previously assumed 'dev' could be null (see line 239)
drivers/iommu/io-pgtable-arm.c
238 VM_BUG_ON((gfp & __GFP_HIGHMEM));
239 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
^^^
We added a NULL check here
240 gfp | __GFP_ZERO, order);
241 if (!p)
242 return NULL;
243
244 pages = page_address(p);
245 if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
246 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
^^^
But dma_map_single() can't take a NULL argument either.
247 if (dma_mapping_error(dev, dma))
248 goto out_free;
I don't know why this warning is only showing up now, almost a year
later. Sorry about that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest
@ 2019-04-10 9:34 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-04-10 9:34 UTC (permalink / raw)
To: jean-philippe.brucker; +Cc: iommu, Robin Murphy
Hello Jean-Philippe Brucker,
This is a semi-automatic email about new static checker warnings.
The patch fac83d29d954: "iommu/io-pgtable-arm: Fix pgtable allocation
in selftest" from Jun 18, 2018, leads to the following Smatch
complaint:
drivers/iommu/io-pgtable-arm.c:246 __arm_lpae_alloc_pages()
error: we previously assumed 'dev' could be null (see line 239)
drivers/iommu/io-pgtable-arm.c
238 VM_BUG_ON((gfp & __GFP_HIGHMEM));
239 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
^^^
We added a NULL check here
240 gfp | __GFP_ZERO, order);
241 if (!p)
242 return NULL;
243
244 pages = page_address(p);
245 if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
246 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
^^^
But dma_map_single() can't take a NULL argument either.
247 if (dma_mapping_error(dev, dma))
248 goto out_free;
I don't know why this warning is only showing up now, almost a year
later. Sorry about that.
regards,
dan carpenter
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest
@ 2019-04-10 9:44 ` Robin Murphy
0 siblings, 0 replies; 6+ messages in thread
From: Robin Murphy @ 2019-04-10 9:44 UTC (permalink / raw)
To: Dan Carpenter, jean-philippe.brucker-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Hi Dan,
On 10/04/2019 10:34, Dan Carpenter wrote:
> Hello Jean-Philippe Brucker,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch fac83d29d954: "iommu/io-pgtable-arm: Fix pgtable allocation
> in selftest" from Jun 18, 2018, leads to the following Smatch
> complaint:
>
> drivers/iommu/io-pgtable-arm.c:246 __arm_lpae_alloc_pages()
> error: we previously assumed 'dev' could be null (see line 239)
>
> drivers/iommu/io-pgtable-arm.c
> 238 VM_BUG_ON((gfp & __GFP_HIGHMEM));
> 239 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
> ^^^
> We added a NULL check here
>
> 240 gfp | __GFP_ZERO, order);
> 241 if (!p)
> 242 return NULL;
> 243
> 244 pages = page_address(p);
> 245 if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
The selftests *should* always set this quirk such that they never get to
the DMA mapping calls (that was one of the reasons for implementing
things that way) - I guess that might be a bit too sneaky for Smatch,
but I can take a look to double-check that the flow is working correctly
such that this really is a false-positive.
Robin.
> 246 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
> ^^^
> But dma_map_single() can't take a NULL argument either.
>
> 247 if (dma_mapping_error(dev, dma))
> 248 goto out_free;
>
> I don't know why this warning is only showing up now, almost a year
> later. Sorry about that.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest
@ 2019-04-10 9:44 ` Robin Murphy
0 siblings, 0 replies; 6+ messages in thread
From: Robin Murphy @ 2019-04-10 9:44 UTC (permalink / raw)
To: Dan Carpenter, jean-philippe.brucker; +Cc: iommu
Hi Dan,
On 10/04/2019 10:34, Dan Carpenter wrote:
> Hello Jean-Philippe Brucker,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch fac83d29d954: "iommu/io-pgtable-arm: Fix pgtable allocation
> in selftest" from Jun 18, 2018, leads to the following Smatch
> complaint:
>
> drivers/iommu/io-pgtable-arm.c:246 __arm_lpae_alloc_pages()
> error: we previously assumed 'dev' could be null (see line 239)
>
> drivers/iommu/io-pgtable-arm.c
> 238 VM_BUG_ON((gfp & __GFP_HIGHMEM));
> 239 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
> ^^^
> We added a NULL check here
>
> 240 gfp | __GFP_ZERO, order);
> 241 if (!p)
> 242 return NULL;
> 243
> 244 pages = page_address(p);
> 245 if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
The selftests *should* always set this quirk such that they never get to
the DMA mapping calls (that was one of the reasons for implementing
things that way) - I guess that might be a bit too sneaky for Smatch,
but I can take a look to double-check that the flow is working correctly
such that this really is a false-positive.
Robin.
> 246 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
> ^^^
> But dma_map_single() can't take a NULL argument either.
>
> 247 if (dma_mapping_error(dev, dma))
> 248 goto out_free;
>
> I don't know why this warning is only showing up now, almost a year
> later. Sorry about that.
>
> regards,
> dan carpenter
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread[parent not found: <782a999e-ce70-bd8e-5a7b-cd320ce98144-5wv7dgnIgG8@public.gmane.org>]
* Re: [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest
@ 2019-04-10 10:02 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-04-10 10:02 UTC (permalink / raw)
To: Robin Murphy
Cc: jean-philippe.brucker-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
On Wed, Apr 10, 2019 at 10:44:03AM +0100, Robin Murphy wrote:
> Hi Dan,
>
> On 10/04/2019 10:34, Dan Carpenter wrote:
> > Hello Jean-Philippe Brucker,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch fac83d29d954: "iommu/io-pgtable-arm: Fix pgtable allocation
> > in selftest" from Jun 18, 2018, leads to the following Smatch
> > complaint:
> >
> > drivers/iommu/io-pgtable-arm.c:246 __arm_lpae_alloc_pages()
> > error: we previously assumed 'dev' could be null (see line 239)
> >
> > drivers/iommu/io-pgtable-arm.c
> > 238 VM_BUG_ON((gfp & __GFP_HIGHMEM));
> > 239 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
> > ^^^
> > We added a NULL check here
> >
> > 240 gfp | __GFP_ZERO, order);
> > 241 if (!p)
> > 242 return NULL;
> > 243
> > 244 pages = page_address(p);
> > 245 if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
>
> The selftests *should* always set this quirk such that they never get to the
> DMA mapping calls (that was one of the reasons for implementing things that
> way) - I guess that might be a bit too sneaky for Smatch, but I can take a
> look to double-check that the flow is working correctly such that this
> really is a false-positive.
Ah, thanks.
I have a plan for fixing these false positives caused by not tracking
bit masks. It's not that complicated to fix, it just takes time to
write the code.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest
@ 2019-04-10 10:02 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-04-10 10:02 UTC (permalink / raw)
To: Robin Murphy; +Cc: jean-philippe.brucker, iommu
On Wed, Apr 10, 2019 at 10:44:03AM +0100, Robin Murphy wrote:
> Hi Dan,
>
> On 10/04/2019 10:34, Dan Carpenter wrote:
> > Hello Jean-Philippe Brucker,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch fac83d29d954: "iommu/io-pgtable-arm: Fix pgtable allocation
> > in selftest" from Jun 18, 2018, leads to the following Smatch
> > complaint:
> >
> > drivers/iommu/io-pgtable-arm.c:246 __arm_lpae_alloc_pages()
> > error: we previously assumed 'dev' could be null (see line 239)
> >
> > drivers/iommu/io-pgtable-arm.c
> > 238 VM_BUG_ON((gfp & __GFP_HIGHMEM));
> > 239 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
> > ^^^
> > We added a NULL check here
> >
> > 240 gfp | __GFP_ZERO, order);
> > 241 if (!p)
> > 242 return NULL;
> > 243
> > 244 pages = page_address(p);
> > 245 if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
>
> The selftests *should* always set this quirk such that they never get to the
> DMA mapping calls (that was one of the reasons for implementing things that
> way) - I guess that might be a bit too sneaky for Smatch, but I can take a
> look to double-check that the flow is working correctly such that this
> really is a false-positive.
Ah, thanks.
I have a plan for fixing these false positives caused by not tracking
bit masks. It's not that complicated to fix, it just takes time to
write the code.
regards,
dan carpenter
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-10 10:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-10 9:34 [bug report] iommu/io-pgtable-arm: Fix pgtable allocation in selftest Dan Carpenter
2019-04-10 9:34 ` Dan Carpenter
2019-04-10 9:44 ` Robin Murphy
2019-04-10 9:44 ` Robin Murphy
[not found] ` <782a999e-ce70-bd8e-5a7b-cd320ce98144-5wv7dgnIgG8@public.gmane.org>
2019-04-10 10:02 ` Dan Carpenter
2019-04-10 10:02 ` 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.