* [PATCH] dma-mapping: remove an unnecessary NULL check
@ 2019-04-24 14:24 Dan Carpenter
2019-04-24 14:27 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2019-04-24 14:24 UTC (permalink / raw)
To: Christoph Hellwig, Ian Abbott, Jiri Slaby
Cc: Marek Szyprowski, Robin Murphy, iommu,
kernel-janitors@vger.kernel.org H Hartley Sweeten,
Greg Kroah-Hartman, linux-kernel
We already dereferenced "dev" when we called get_dma_ops() so this NULL
check is too late. We're not supposed to pass NULL "dev" pointers to
dma_alloc_attrs().
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
There are still at least two drivers which do pass a NULL unfortunately.
drivers/staging/comedi/drivers/comedi_isadma.c:195 comedi_isadma_alloc() error: NULL dereference inside function 'dma_alloc_coherent()'
drivers/staging/comedi/drivers/comedi_isadma.c:227 comedi_isadma_free() error: NULL dereference inside function 'dma_free_coherent()'
drivers/tty/synclink.c:3667 mgsl_alloc_buffer_list_memory() error: NULL dereference inside function 'dma_alloc_coherent()'
drivers/tty/synclink.c:3738 mgsl_free_buffer_list_memory() error: NULL dereference inside function 'dma_free_coherent()'
drivers/tty/synclink.c:3777 mgsl_alloc_frame_memory() error: NULL dereference inside function 'dma_alloc_coherent()'
drivers/tty/synclink.c:3811 mgsl_free_frame_memory() error: NULL dereference inside function 'dma_free_coherent()'
If I remember right there are one or two others which sometimes pass a
NULL, but it's harder to grep for those warnings.
kernel/dma/mapping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 685a53f2a793..f7afdadb6770 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -244,7 +244,7 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
const struct dma_map_ops *ops = get_dma_ops(dev);
void *cpu_addr;
- WARN_ON_ONCE(dev && !dev->coherent_dma_mask);
+ WARN_ON_ONCE(!dev->coherent_dma_mask);
if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr))
return cpu_addr;
--
2.18.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dma-mapping: remove an unnecessary NULL check
2019-04-24 14:24 [PATCH] dma-mapping: remove an unnecessary NULL check Dan Carpenter
@ 2019-04-24 14:27 ` Christoph Hellwig
2019-04-25 14:13 ` Ian Abbott
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2019-04-24 14:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: Christoph Hellwig, Ian Abbott, Jiri Slaby, Marek Szyprowski,
Robin Murphy, iommu,
kernel-janitors@vger.kernel.org H Hartley Sweeten,
Greg Kroah-Hartman, linux-kernel
On Wed, Apr 24, 2019 at 05:24:37PM +0300, Dan Carpenter wrote:
> We already dereferenced "dev" when we called get_dma_ops() so this NULL
> check is too late. We're not supposed to pass NULL "dev" pointers to
> dma_alloc_attrs().
Thanks, applied to the dma-mapping for-next tree.
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> There are still at least two drivers which do pass a NULL unfortunately.
>
> drivers/staging/comedi/drivers/comedi_isadma.c:195 comedi_isadma_alloc() error: NULL dereference inside function 'dma_alloc_coherent()'
> drivers/staging/comedi/drivers/comedi_isadma.c:227 comedi_isadma_free() error: NULL dereference inside function 'dma_free_coherent()'
This is staging code. Per official decree from Linus we can just
ignore it, and I tend to do so to keep my sanity.
> drivers/tty/synclink.c:3667 mgsl_alloc_buffer_list_memory() error: NULL dereference inside function 'dma_alloc_coherent()'
> drivers/tty/synclink.c:3738 mgsl_free_buffer_list_memory() error: NULL dereference inside function 'dma_free_coherent()'
> drivers/tty/synclink.c:3777 mgsl_alloc_frame_memory() error: NULL dereference inside function 'dma_alloc_coherent()'
> drivers/tty/synclink.c:3811 mgsl_free_frame_memory() error: NULL dereference inside function 'dma_free_coherent()'
The !PCI case there is dead since I removed PCI support a while ago.
Looks like it is still too convoluted for static checkers to notice that,
though.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dma-mapping: remove an unnecessary NULL check
2019-04-24 14:27 ` Christoph Hellwig
@ 2019-04-25 14:13 ` Ian Abbott
2019-04-25 14:18 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Ian Abbott @ 2019-04-25 14:13 UTC (permalink / raw)
To: Christoph Hellwig, Dan Carpenter
Cc: Jiri Slaby, Marek Szyprowski, Robin Murphy, iommu,
kernel-janitors@vger.kernel.org H Hartley Sweeten,
Greg Kroah-Hartman, linux-kernel
On 24/04/2019 15:27, Christoph Hellwig wrote:
> On Wed, Apr 24, 2019 at 05:24:37PM +0300, Dan Carpenter wrote:
>> We already dereferenced "dev" when we called get_dma_ops() so this NULL
>> check is too late. We're not supposed to pass NULL "dev" pointers to
>> dma_alloc_attrs().
>
> Thanks, applied to the dma-mapping for-next tree.
>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> There are still at least two drivers which do pass a NULL unfortunately.
>>
>> drivers/staging/comedi/drivers/comedi_isadma.c:195 comedi_isadma_alloc() error: NULL dereference inside function 'dma_alloc_coherent()'
>> drivers/staging/comedi/drivers/comedi_isadma.c:227 comedi_isadma_free() error: NULL dereference inside function 'dma_free_coherent()'
>
> This is staging code. Per official decree from Linus we can just
> ignore it, and I tend to do so to keep my sanity.
So for comedi_isadma, we can just replace the NULL with a pointer to a
static dummy device with a 24-bit coherent mask?
--
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dma-mapping: remove an unnecessary NULL check
2019-04-25 14:13 ` Ian Abbott
@ 2019-04-25 14:18 ` Christoph Hellwig
2019-04-25 14:31 ` Ian Abbott
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2019-04-25 14:18 UTC (permalink / raw)
To: Ian Abbott
Cc: Christoph Hellwig, Dan Carpenter, Jiri Slaby, Marek Szyprowski,
Robin Murphy, iommu,
kernel-janitors@vger.kernel.org H Hartley Sweeten,
Greg Kroah-Hartman, linux-kernel
On Thu, Apr 25, 2019 at 03:13:49PM +0100, Ian Abbott wrote:
> So for comedi_isadma, we can just replace the NULL with a pointer to a
> static dummy device with a 24-bit coherent mask?
It should be converted to a proper isa_driver, so that it gets a real
struct device.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dma-mapping: remove an unnecessary NULL check
2019-04-25 14:18 ` Christoph Hellwig
@ 2019-04-25 14:31 ` Ian Abbott
2019-04-25 14:32 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Ian Abbott @ 2019-04-25 14:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Dan Carpenter, Jiri Slaby, Marek Szyprowski, Robin Murphy, iommu,
kernel-janitors@vger.kernel.org H Hartley Sweeten,
Greg Kroah-Hartman, linux-kernel
On 25/04/2019 15:18, Christoph Hellwig wrote:
> On Thu, Apr 25, 2019 at 03:13:49PM +0100, Ian Abbott wrote:
>> So for comedi_isadma, we can just replace the NULL with a pointer to a
>> static dummy device with a 24-bit coherent mask?
>
> It should be converted to a proper isa_driver, so that it gets a real
> struct device.
But it will work as a short term solution? comedi_isadma isn't a
driver, but is used by a few comedi drivers for ISA cards.
--
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dma-mapping: remove an unnecessary NULL check
2019-04-25 14:31 ` Ian Abbott
@ 2019-04-25 14:32 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-04-25 14:32 UTC (permalink / raw)
To: Ian Abbott
Cc: Christoph Hellwig, Dan Carpenter, Jiri Slaby, Marek Szyprowski,
Robin Murphy, iommu,
kernel-janitors@vger.kernel.org H Hartley Sweeten,
Greg Kroah-Hartman, linux-kernel
On Thu, Apr 25, 2019 at 03:31:01PM +0100, Ian Abbott wrote:
> On 25/04/2019 15:18, Christoph Hellwig wrote:
>> On Thu, Apr 25, 2019 at 03:13:49PM +0100, Ian Abbott wrote:
>>> So for comedi_isadma, we can just replace the NULL with a pointer to a
>>> static dummy device with a 24-bit coherent mask?
>>
>> It should be converted to a proper isa_driver, so that it gets a real
>> struct device.
>
> But it will work as a short term solution? comedi_isadma isn't a driver,
> but is used by a few comedi drivers for ISA cards.
Then convert the drivers properly and pass on the device from them.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-25 14:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 14:24 [PATCH] dma-mapping: remove an unnecessary NULL check Dan Carpenter
2019-04-24 14:27 ` Christoph Hellwig
2019-04-25 14:13 ` Ian Abbott
2019-04-25 14:18 ` Christoph Hellwig
2019-04-25 14:31 ` Ian Abbott
2019-04-25 14:32 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).