From a0d78193dcf2bfa7c3d169c55a7a30a5a89fca7a Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Fri, 17 Mar 2017 12:48:15 -0600 Subject: 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 Signed-off-by: Logan Gunthorpe Reviewed-by: Logan Gunthorpe Reviewed-by: Leon Romanovsky Signed-off-by: Greg Kroah-Hartman --- drivers/infiniband/core/ucm.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'drivers/infiniband') 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, -- cgit v1.2.3 From 985087157cebd2e710c3e65d8c3eabde4c591f1c Mon Sep 17 00:00:00 2001 From: Logan Gunthorpe Date: Fri, 17 Mar 2017 12:48:16 -0600 Subject: infiniband: utilize the new cdev_set_parent function This replaces the suspect looking cdev.kobj.parent lines with the equivalent cdev_set_parent function. This is a straightforward change that's largely cosmetic but it does push the kobj.parent ownership into char_dev.c where it belongs. Signed-off-by: Logan Gunthorpe Signed-off-by: Greg Kroah-Hartman --- drivers/infiniband/core/user_mad.c | 4 ++-- drivers/infiniband/core/uverbs_main.c | 2 +- drivers/infiniband/hw/hfi1/device.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/infiniband') diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index aca7ff7abedc..25071b880c94 100644 --- a/drivers/infiniband/core/user_mad.c +++ b/drivers/infiniband/core/user_mad.c @@ -1183,7 +1183,7 @@ static int ib_umad_init_port(struct ib_device *device, int port_num, cdev_init(&port->cdev, &umad_fops); port->cdev.owner = THIS_MODULE; - port->cdev.kobj.parent = &umad_dev->kobj; + cdev_set_parent(&port->cdev, &umad_dev->kobj); kobject_set_name(&port->cdev.kobj, "umad%d", port->dev_num); if (cdev_add(&port->cdev, base, 1)) goto err_cdev; @@ -1202,7 +1202,7 @@ static int ib_umad_init_port(struct ib_device *device, int port_num, base += IB_UMAD_MAX_PORTS; cdev_init(&port->sm_cdev, &umad_sm_fops); port->sm_cdev.owner = THIS_MODULE; - port->sm_cdev.kobj.parent = &umad_dev->kobj; + cdev_set_parent(&port->sm_cdev, &umad_dev->kobj); kobject_set_name(&port->sm_cdev.kobj, "issm%d", port->dev_num); if (cdev_add(&port->sm_cdev, base, 1)) goto err_sm_cdev; diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c index 35c788a32e26..f02d7b9cebea 100644 --- a/drivers/infiniband/core/uverbs_main.c +++ b/drivers/infiniband/core/uverbs_main.c @@ -1189,7 +1189,7 @@ static void ib_uverbs_add_one(struct ib_device *device) cdev_init(&uverbs_dev->cdev, NULL); uverbs_dev->cdev.owner = THIS_MODULE; uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops : &uverbs_fops; - uverbs_dev->cdev.kobj.parent = &uverbs_dev->kobj; + cdev_set_parent(&uverbs_dev->cdev, &uverbs_dev->kobj); kobject_set_name(&uverbs_dev->cdev.kobj, "uverbs%d", uverbs_dev->devnum); if (cdev_add(&uverbs_dev->cdev, base, 1)) goto err_cdev; diff --git a/drivers/infiniband/hw/hfi1/device.c b/drivers/infiniband/hw/hfi1/device.c index bf64b5a7bfd7..bbb6069dec2a 100644 --- a/drivers/infiniband/hw/hfi1/device.c +++ b/drivers/infiniband/hw/hfi1/device.c @@ -69,7 +69,7 @@ int hfi1_cdev_init(int minor, const char *name, cdev_init(cdev, fops); cdev->owner = THIS_MODULE; - cdev->kobj.parent = parent; + cdev_set_parent(cdev, parent); kobject_set_name(&cdev->kobj, name); ret = cdev_add(cdev, dev, 1); -- cgit v1.2.3