dmaengine: fail device registration if channel registration fails

Atsushi points out:
"If alloc_percpu or kzalloc failed, chan_id does not match with its
position in device->channels list.

And above "continue" looks buggy anyway.  Keeping incomplete channels
in device->channels list looks very dangerous..."

Also, fix up leakage of idr_ref in the idr_pre_get() and channel init
fail cases.

Reported-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
This commit is contained in:
Dan Williams 2009-03-25 09:13:23 -07:00
parent 041b62374c
commit 257b17ca03

View file

@ -602,6 +602,24 @@ void dmaengine_put(void)
} }
EXPORT_SYMBOL(dmaengine_put); EXPORT_SYMBOL(dmaengine_put);
static int get_dma_id(struct dma_device *device)
{
int rc;
idr_retry:
if (!idr_pre_get(&dma_idr, GFP_KERNEL))
return -ENOMEM;
mutex_lock(&dma_list_mutex);
rc = idr_get_new(&dma_idr, NULL, &device->dev_id);
mutex_unlock(&dma_list_mutex);
if (rc == -EAGAIN)
goto idr_retry;
else if (rc != 0)
return rc;
return 0;
}
/** /**
* dma_async_device_register - registers DMA devices found * dma_async_device_register - registers DMA devices found
* @device: &dma_device * @device: &dma_device
@ -640,27 +658,25 @@ int dma_async_device_register(struct dma_device *device)
idr_ref = kmalloc(sizeof(*idr_ref), GFP_KERNEL); idr_ref = kmalloc(sizeof(*idr_ref), GFP_KERNEL);
if (!idr_ref) if (!idr_ref)
return -ENOMEM; return -ENOMEM;
atomic_set(idr_ref, 0); rc = get_dma_id(device);
idr_retry: if (rc != 0) {
if (!idr_pre_get(&dma_idr, GFP_KERNEL)) kfree(idr_ref);
return -ENOMEM;
mutex_lock(&dma_list_mutex);
rc = idr_get_new(&dma_idr, NULL, &device->dev_id);
mutex_unlock(&dma_list_mutex);
if (rc == -EAGAIN)
goto idr_retry;
else if (rc != 0)
return rc; return rc;
}
atomic_set(idr_ref, 0);
/* represent channels in sysfs. Probably want devs too */ /* represent channels in sysfs. Probably want devs too */
list_for_each_entry(chan, &device->channels, device_node) { list_for_each_entry(chan, &device->channels, device_node) {
rc = -ENOMEM;
chan->local = alloc_percpu(typeof(*chan->local)); chan->local = alloc_percpu(typeof(*chan->local));
if (chan->local == NULL) if (chan->local == NULL)
continue; goto err_out;
chan->dev = kzalloc(sizeof(*chan->dev), GFP_KERNEL); chan->dev = kzalloc(sizeof(*chan->dev), GFP_KERNEL);
if (chan->dev == NULL) { if (chan->dev == NULL) {
free_percpu(chan->local); free_percpu(chan->local);
continue; chan->local = NULL;
goto err_out;
} }
chan->chan_id = chancnt++; chan->chan_id = chancnt++;
@ -677,6 +693,8 @@ int dma_async_device_register(struct dma_device *device)
if (rc) { if (rc) {
free_percpu(chan->local); free_percpu(chan->local);
chan->local = NULL; chan->local = NULL;
kfree(chan->dev);
atomic_dec(idr_ref);
goto err_out; goto err_out;
} }
chan->client_count = 0; chan->client_count = 0;
@ -707,6 +725,15 @@ int dma_async_device_register(struct dma_device *device)
return 0; return 0;
err_out: err_out:
/* if we never registered a channel just release the idr */
if (atomic_read(idr_ref) == 0) {
mutex_lock(&dma_list_mutex);
idr_remove(&dma_idr, device->dev_id);
mutex_unlock(&dma_list_mutex);
kfree(idr_ref);
return rc;
}
list_for_each_entry(chan, &device->channels, device_node) { list_for_each_entry(chan, &device->channels, device_node) {
if (chan->local == NULL) if (chan->local == NULL)
continue; continue;