Commit ac1ddc58 authored by Logan Gunthorpe's avatar Logan Gunthorpe Committed by Greg Kroah-Hartman

scsi: utilize new cdev_device_add helper function

This driver did not set kobj.parent so it likely suffered from
a potential use after free race if the user unregistered the
device while it was in use.

This was not so straightforward a conversion but I think this patch
cleans up its probe's error path significantly.

This patch adds device_initialize, which is required for
cdev_device_add. Then it switches to put_device instead of kfree as
recommended by device_initialize's documentation. This removes a lot
from the error path which was already in __remove.
A couple things needed to be re-ordered to be entirely correct, though.
ida_remove is also moved out of __remove and into unregister to
simplify things and follow the pattern other devices are using.

This also drop an extra unnecessary get_device/put_device in the code.
Signed-off-by: default avatarLogan Gunthorpe <logang@deltatee.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent d5ed9177
...@@ -400,9 +400,6 @@ static void __remove(struct device *dev) ...@@ -400,9 +400,6 @@ static void __remove(struct device *dev)
kfree(oud->odi.osdname); kfree(oud->odi.osdname);
if (oud->cdev.owner)
cdev_del(&oud->cdev);
osd_dev_fini(&oud->od); osd_dev_fini(&oud->od);
scsi_device_put(scsi_device); scsi_device_put(scsi_device);
...@@ -411,7 +408,6 @@ static void __remove(struct device *dev) ...@@ -411,7 +408,6 @@ static void __remove(struct device *dev)
if (oud->disk) if (oud->disk)
put_disk(oud->disk); put_disk(oud->disk);
ida_remove(&osd_minor_ida, oud->minor);
kfree(oud); kfree(oud);
} }
...@@ -446,8 +442,20 @@ static int osd_probe(struct device *dev) ...@@ -446,8 +442,20 @@ static int osd_probe(struct device *dev)
if (NULL == oud) if (NULL == oud)
goto err_retract_minor; goto err_retract_minor;
/* class device member */
device_initialize(&oud->class_dev);
dev_set_drvdata(dev, oud); dev_set_drvdata(dev, oud);
oud->minor = minor; oud->minor = minor;
oud->class_dev.devt = MKDEV(SCSI_OSD_MAJOR, oud->minor);
oud->class_dev.class = &osd_uld_class;
oud->class_dev.parent = dev;
oud->class_dev.release = __remove;
/* hold one more reference to the scsi_device that will get released
* in __release, in case a logout is happening while fs is mounted
*/
scsi_device_get(scsi_device);
osd_dev_init(&oud->od, scsi_device);
/* allocate a disk and set it up */ /* allocate a disk and set it up */
/* FIXME: do we need this since sg has already done that */ /* FIXME: do we need this since sg has already done that */
...@@ -461,59 +469,34 @@ static int osd_probe(struct device *dev) ...@@ -461,59 +469,34 @@ static int osd_probe(struct device *dev)
sprintf(disk->disk_name, "osd%d", oud->minor); sprintf(disk->disk_name, "osd%d", oud->minor);
oud->disk = disk; oud->disk = disk;
/* hold one more reference to the scsi_device that will get released
* in __release, in case a logout is happening while fs is mounted
*/
scsi_device_get(scsi_device);
osd_dev_init(&oud->od, scsi_device);
/* Detect the OSD Version */ /* Detect the OSD Version */
error = __detect_osd(oud); error = __detect_osd(oud);
if (error) { if (error) {
OSD_ERR("osd detection failed, non-compatible OSD device\n"); OSD_ERR("osd detection failed, non-compatible OSD device\n");
goto err_put_disk; goto err_free_osd;
} }
/* init the char-device for communication with user-mode */ /* init the char-device for communication with user-mode */
cdev_init(&oud->cdev, &osd_fops); cdev_init(&oud->cdev, &osd_fops);
oud->cdev.owner = THIS_MODULE; oud->cdev.owner = THIS_MODULE;
error = cdev_add(&oud->cdev,
MKDEV(SCSI_OSD_MAJOR, oud->minor), 1);
if (error) {
OSD_ERR("cdev_add failed\n");
goto err_put_disk;
}
/* class device member */
oud->class_dev.devt = oud->cdev.dev;
oud->class_dev.class = &osd_uld_class;
oud->class_dev.parent = dev;
oud->class_dev.release = __remove;
error = dev_set_name(&oud->class_dev, "%s", disk->disk_name); error = dev_set_name(&oud->class_dev, "%s", disk->disk_name);
if (error) { if (error) {
OSD_ERR("dev_set_name failed => %d\n", error); OSD_ERR("dev_set_name failed => %d\n", error);
goto err_put_cdev; goto err_free_osd;
} }
error = device_register(&oud->class_dev); error = cdev_device_add(&oud->cdev, &oud->class_dev);
if (error) { if (error) {
OSD_ERR("device_register failed => %d\n", error); OSD_ERR("device_register failed => %d\n", error);
goto err_put_cdev; goto err_free_osd;
} }
get_device(&oud->class_dev);
OSD_INFO("osd_probe %s\n", disk->disk_name); OSD_INFO("osd_probe %s\n", disk->disk_name);
return 0; return 0;
err_put_cdev:
cdev_del(&oud->cdev);
err_put_disk:
scsi_device_put(scsi_device);
put_disk(disk);
err_free_osd: err_free_osd:
dev_set_drvdata(dev, NULL); put_device(&oud->class_dev);
kfree(oud);
err_retract_minor: err_retract_minor:
ida_remove(&osd_minor_ida, minor); ida_remove(&osd_minor_ida, minor);
return error; return error;
...@@ -530,9 +513,10 @@ static int osd_remove(struct device *dev) ...@@ -530,9 +513,10 @@ static int osd_remove(struct device *dev)
scsi_device); scsi_device);
} }
device_unregister(&oud->class_dev); cdev_device_del(&oud->cdev, &oud->class_dev);
ida_remove(&osd_minor_ida, oud->minor);
put_device(&oud->class_dev); put_device(&oud->class_dev);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment