diff options
| author | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> | 2017-03-17 12:48:15 -0600 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2017-03-21 06:44:33 +0100 |
| commit | a0d78193dcf2bfa7c3d169c55a7a30a5a89fca7a (patch) | |
| tree | 1b2c23c01c1a415cf4bad27b0e30d2e6cae3b074 | |
| parent | 1c1d152cc5ac3a67d199728a9ba0b4f54a498f8b (diff) | |
IB/ucm: utilize new cdev_device_add helper function
The use after free is not triggerable here because the cdev holds
the module lock and the only device_unregister is only triggered by
module unload, however make the change for consistency.
To make this work the cdev_del needs to move out of the struct device
release function.
This cleans up the error path significantly and thus also fixes a minor
bug where the devnum would not be released if cdev_add failed.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
| -rw-r--r-- | drivers/infiniband/core/ucm.c | 35 |
1 files changed, 18 insertions, 17 deletions
diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c index cc0d51fb06e3..d15efa4be9ab 100644 --- a/drivers/infiniband/core/ucm.c +++ b/drivers/infiniband/core/ucm.c @@ -1205,12 +1205,15 @@ static void ib_ucm_release_dev(struct device *dev) struct ib_ucm_device *ucm_dev; ucm_dev = container_of(dev, struct ib_ucm_device, dev); - cdev_del(&ucm_dev->cdev); + kfree(ucm_dev); +} + +static void ib_ucm_free_dev(struct ib_ucm_device *ucm_dev) +{ if (ucm_dev->devnum < IB_UCM_MAX_DEVICES) clear_bit(ucm_dev->devnum, dev_map); else clear_bit(ucm_dev->devnum - IB_UCM_MAX_DEVICES, overflow_map); - kfree(ucm_dev); } static const struct file_operations ucm_fops = { @@ -1266,7 +1269,9 @@ static void ib_ucm_add_one(struct ib_device *device) if (!ucm_dev) return; + device_initialize(&ucm_dev->dev); ucm_dev->ib_dev = device; + ucm_dev->dev.release = ib_ucm_release_dev; devnum = find_first_zero_bit(dev_map, IB_UCM_MAX_DEVICES); if (devnum >= IB_UCM_MAX_DEVICES) { @@ -1286,16 +1291,14 @@ static void ib_ucm_add_one(struct ib_device *device) cdev_init(&ucm_dev->cdev, &ucm_fops); ucm_dev->cdev.owner = THIS_MODULE; kobject_set_name(&ucm_dev->cdev.kobj, "ucm%d", ucm_dev->devnum); - if (cdev_add(&ucm_dev->cdev, base, 1)) - goto err; ucm_dev->dev.class = &cm_class; ucm_dev->dev.parent = device->dev.parent; - ucm_dev->dev.devt = ucm_dev->cdev.dev; - ucm_dev->dev.release = ib_ucm_release_dev; + ucm_dev->dev.devt = base; + dev_set_name(&ucm_dev->dev, "ucm%d", ucm_dev->devnum); - if (device_register(&ucm_dev->dev)) - goto err_cdev; + if (cdev_device_add(&ucm_dev->cdev, &ucm_dev->dev)) + goto err_devnum; if (device_create_file(&ucm_dev->dev, &dev_attr_ibdev)) goto err_dev; @@ -1304,15 +1307,11 @@ static void ib_ucm_add_one(struct ib_device *device) return; err_dev: - device_unregister(&ucm_dev->dev); -err_cdev: - cdev_del(&ucm_dev->cdev); - if (ucm_dev->devnum < IB_UCM_MAX_DEVICES) - clear_bit(devnum, dev_map); - else - clear_bit(devnum, overflow_map); + cdev_device_del(&ucm_dev->cdev, &ucm_dev->dev); +err_devnum: + ib_ucm_free_dev(ucm_dev); err: - kfree(ucm_dev); + put_device(&ucm_dev->dev); return; } @@ -1323,7 +1322,9 @@ static void ib_ucm_remove_one(struct ib_device *device, void *client_data) if (!ucm_dev) return; - device_unregister(&ucm_dev->dev); + cdev_device_del(&ucm_dev->cdev, &ucm_dev->dev); + ib_ucm_free_dev(ucm_dev); + put_device(&ucm_dev->dev); } static CLASS_ATTR_STRING(abi_version, S_IRUGO, |