"dev" cannot be NULL because it is already checked before
calling dma_pool_create().
Signed-off-by: Daeseok Youn <[email protected]>
---
If dev can be NULL, it has NULL deferencing when kmalloc_node()
is called after enabling CONFIG_NUMA.
mm/dmapool.c | 26 +++++++++-----------------
1 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/mm/dmapool.c b/mm/dmapool.c
index c69781e..38dfcdd 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -170,24 +170,16 @@ struct dma_pool *dma_pool_create(const char *name, struct device *dev,
retval->boundary = boundary;
retval->allocation = allocation;
- if (dev) {
- int ret;
+ INIT_LIST_HEAD(&retval->pools);
- mutex_lock(&pools_lock);
- if (list_empty(&dev->dma_pools))
- ret = device_create_file(dev, &dev_attr_pools);
- else
- ret = 0;
- /* note: not currently insisting "name" be unique */
- if (!ret)
- list_add(&retval->pools, &dev->dma_pools);
- else {
- kfree(retval);
- retval = NULL;
- }
- mutex_unlock(&pools_lock);
+ mutex_lock(&pools_lock);
+ if (list_empty(&dev->dma_pools) &&
+ device_create_file(dev, &dev_attr_pools)) {
+ kfree(retval);
+ return NULL;
} else
- INIT_LIST_HEAD(&retval->pools);
+ list_add(&retval->pools, &dev->dma_pools);
+ mutex_unlock(&pools_lock);
return retval;
}
--
1.7.4.4
On Tue, 29 Apr 2014 11:53:10 +0900 Daeseok Youn <[email protected]> wrote:
> "dev" cannot be NULL because it is already checked before
> calling dma_pool_create().
>
> Signed-off-by: Daeseok Youn <[email protected]>
> ---
> If dev can be NULL, it has NULL deferencing when kmalloc_node()
> is called after enabling CONFIG_NUMA.
hm, this is unclear.
The code which handles the dev==NULL case was obviously put there
deliberately, presumably with the intention of permitting drivers to
call dma_pool_create() without a device*. This code is very old.
A lot of drivers call dma_pool_create() (I doubt if you audited all of
them!) and perhaps there are some which use this feature and have never
been run on NUMA hardware.
I think I'll apply the patch anyway because such drivers (if they
exist) probably need some attending to.
I rewrote the changelog thusly:
: "dev" cannot be NULL because it is already checked before calling
: dma_pool_create().
:
: If dev ever was NULL, the code would oops in dev_to_node() after enabling
: CONFIG_NUMA.
:
: It is possible that some driver is using dev==NULL and has never been run
: on a NUMA machine. Such a driver is probably outdated, possibly buggy and
: will need some attention if it starts triggering NULL derefs.