[PATCH 1/5] Driver core: driver_find() drops reference before returning

January 24th, 2012 - 01:40 pm ET by Alan Stern | Report spam
As part of the removal of get_driver()/put_driver(), this patch
(as1510) changes driver_find(); it now drops the reference it acquires
before returning. The patch also adjusts all the callers of
driver_find() to remove the now unnecessary calls to put_driver().

In addition, the patch adds a warning to driver_find(): Callers must
make sure the driver they are searching for does not get unloaded
while they are using it. This has always been the case; driver_find()
has never prevented a driver from being unregistered or unloaded.
Hence the patch will not introduce any new bugs. The existing callers
all seem to be okay in this respect, however I don't understand the
video drivers well enough to be certain about them.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Kyungmin Park <kyungmin.park@samsung.com>
CC: Andy Walls <awalls@md.metrocast.net>
CC: Martin Schwidefsky <schwidefsky@de.ibm.com>



drivers/base/driver.c | 7 +++++--
drivers/input/gameport/gameport.c | 1 -
drivers/input/serio/serio.c | 1 -
drivers/media/video/cx18/cx18-alsa-main.c | 1 -
drivers/media/video/ivtv/ivtvfb.c | 2 --
drivers/media/video/s5p-fimc/fimc-mdevice.c | 5 +-
drivers/media/video/s5p-tv/mixer_video.c | 1 -
drivers/s390/net/smsgiucv_app.c | 9 ++++--
8 files changed, 10 insertions(+), 17 deletions(-)

Index: usb-3.3/drivers/base/driver.c
usb-3.3.orig/drivers/base/driver.c
+++ usb-3.3/drivers/base/driver.c
@@ -234,7 +234,6 @@ int driver_register(struct device_driver

other = driver_find(drv->name, drv->bus);
if (other) {
- put_driver(other);
printk(KERN_ERR "Error: Driver '%s' is already registered, "
"aborting...", drv->name);
return -EBUSY;
@@ -275,7 +274,9 @@ EXPORT_SYMBOL_GPL(driver_unregister);
* Call kset_find_obj() to iterate over list of drivers on
* a bus to find driver by name. Return driver if found.
*
- * Note that kset_find_obj increments driver's reference count.
+ * This routine provides no locking to prevent the driver it returns
+ * from being unregistered or unloaded while the caller is using it.
+ * The caller is responsible for preventing this.
*/
struct device_driver *driver_find(const char *name, struct bus_type *bus)
{
@@ -283,6 +284,8 @@ struct device_driver *driver_find(const
struct driver_private *priv;

if (k) {
+ /* Drop reference added by kset_find_obj() */
+ kobject_put(k);
priv = to_driver(k);
return priv->driver;
}
Index: usb-3.3/drivers/input/gameport/gameport.c
usb-3.3.orig/drivers/input/gameport/gameport.c
+++ usb-3.3/drivers/input/gameport/gameport.c
@@ -449,7 +449,6 @@ static ssize_t gameport_rebind_driver(st
} else if ((drv = driver_find(buf, &gameport_bus)) != NULL) {
gameport_disconnect_port(gameport);
error = gameport_bind_driver(gameport, to_gameport_driver(drv));
- put_driver(drv);
} else {
error = -EINVAL;
}
Index: usb-3.3/drivers/input/serio/serio.c
usb-3.3.orig/drivers/input/serio/serio.c
+++ usb-3.3/drivers/input/serio/serio.c
@@ -441,7 +441,6 @@ static ssize_t serio_rebind_driver(struc
} else if ((drv = driver_find(buf, &serio_bus)) != NULL) {
serio_disconnect_port(serio);
error = serio_bind_driver(serio, to_serio_driver(drv));
- put_driver(drv);
serio_remove_duplicate_events(serio, SERIO_RESCAN_PORT);
} else {
error = -EINVAL;
Index: usb-3.3/drivers/media/video/cx18/cx18-alsa-main.c
usb-3.3.orig/drivers/media/video/cx18/cx18-alsa-main.c
+++ usb-3.3/drivers/media/video/cx18/cx18-alsa-main.c
@@ -285,7 +285,6 @@ static void __exit cx18_alsa_exit(void)

drv = driver_find("cx18", &pci_bus_type);
ret = driver_for_each_device(drv, NULL, NULL, cx18_alsa_exit_callback);
- put_driver(drv);

cx18_ext_init = NULL;
printk(KERN_INFO "cx18-alsa: module unload complete");
Index: usb-3.3/drivers/media/video/ivtv/ivtvfb.c
usb-3.3.orig/drivers/media/video/ivtv/ivtvfb.c
+++ usb-3.3/drivers/media/video/ivtv/ivtvfb.c
@@ -1293,7 +1293,6 @@ static int __init ivtvfb_init(void)

drv = driver_find("ivtv", &pci_bus_type);
err = driver_for_each_device(drv, NULL, &registered, ivtvfb_callback_init);
- put_driver(drv);
if (!registered) {
printk(KERN_ERR "ivtvfb: no cards found");
return -ENODEV;
@@ -1310,7 +1309,6 @@ static void ivtvfb_cleanup(void)

drv = driver_find("ivtv", &pci_bus_type);
err = driver_for_each_device(drv, NULL, NULL, ivtvfb_callback_cleanup);
- put_driver(drv);
}

module_init(ivtvfb_init);
Index: usb-3.3/drivers/media/video/s5p-fimc/fimc-mdevice.c
usb-3.3.orig/drivers/media/video/s5p-fimc/fimc-mdevice.c
+++ usb-3.3/drivers/media/video/s5p-fimc/fimc-mdevice.c
@@ -344,16 +344,13 @@ static int fimc_md_register_platform_ent
return -ENODEV;
ret = driver_for_each_device(driver, NULL, fmd,
fimc_register_callback);
- put_driver(driver);
if (ret)
return ret;

driver = driver_find(CSIS_DRIVER_NAME, &platform_bus_type);
- if (driver) {
+ if (driver)
ret = driver_for_each_device(driver, NULL, fmd,
csis_register_callback);
- put_driver(driver);
- }
return ret;
}

Index: usb-3.3/drivers/media/video/s5p-tv/mixer_video.c
usb-3.3.orig/drivers/media/video/s5p-tv/mixer_video.c
+++ usb-3.3/drivers/media/video/s5p-tv/mixer_video.c
@@ -58,7 +58,6 @@ static struct v4l2_subdev *find_and_regi
}

done:
- put_driver(drv);
return sd;
}

Index: usb-3.3/drivers/s390/net/smsgiucv_app.c
usb-3.3.orig/drivers/s390/net/smsgiucv_app.c
+++ usb-3.3/drivers/s390/net/smsgiucv_app.c
@@ -168,7 +168,7 @@ static int __init smsgiucv_app_init(void
rc = dev_set_name(smsg_app_dev, KMSG_COMPONENT);
if (rc) {
kfree(smsg_app_dev);
- goto fail_put_driver;
+ goto fail;
}
smsg_app_dev->bus = &iucv_bus;
smsg_app_dev->parent = iucv_root;
@@ -177,7 +177,7 @@ static int __init smsgiucv_app_init(void
rc = device_register(smsg_app_dev);
if (rc) {
put_device(smsg_app_dev);
- goto fail_put_driver;
+ goto fail;
}

/* convert sender to uppercase characters */
@@ -191,12 +191,11 @@ static int __init smsgiucv_app_init(void
rc = smsg_register_callback(SMSG_PREFIX, smsg_app_callback);
if (rc) {
device_unregister(smsg_app_dev);
- goto fail_put_driver;
+ goto fail;
}

rc = 0;
-fail_put_driver:
- put_driver(smsgiucv_drv);
+fail:
return rc;
}
module_init(smsgiucv_app_init);

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
email Follow the discussionReplies 3 repliesReplies Make a reply

Replies

#1 Andy Walls
January 24th, 2012 - 09:00 pm ET | Report spam
Alan Stern wrote:

As part of the removal of get_driver()/put_driver(), this patch
(as1510) changes driver_find(); it now drops the reference it acquires
before returning. The patch also adjusts all the callers of
driver_find() to remove the now unnecessary calls to put_driver().

In addition, the patch adds a warning to driver_find(): Callers must
make sure the driver they are searching for does not get unloaded
while they are using it. This has always been the case; driver_find()
has never prevented a driver from being unregistered or unloaded.
Hence the patch will not introduce any new bugs. The existing callers
all seem to be okay in this respect, however I don't understand the
video drivers well enough to be certain about them.

Signed-off-by: Alan Stern
CC: Dmitry Torokhov
CC: Kyungmin Park
CC: Andy Walls
CC: Martin Schwidefsky



drivers/base/driver.c | 7 +++++--
drivers/input/gameport/gameport.c | 1 -
drivers/input/serio/serio.c | 1 -
drivers/media/video/cx18/cx18-alsa-main.c | 1 -
drivers/media/video/ivtv/ivtvfb.c | 2 --
drivers/media/video/s5p-fimc/fimc-mdevice.c | 5 +-
drivers/media/video/s5p-tv/mixer_video.c | 1 -
drivers/s390/net/smsgiucv_app.c | 9 ++++--
8 files changed, 10 insertions(+), 17 deletions(-)

Index: usb-3.3/drivers/base/driver.c
usb-3.3.orig/drivers/base/driver.c


+++ usb-3.3/drivers/base/driver.c
@@ -234,7 +234,6 @@ int driver_register(struct device_driver

other = driver_find(drv->name, drv->bus);
if (other) {
- put_driver(other);
printk(KERN_ERR "Error: Driver '%s' is already registered, "
"aborting...", drv->name);
return -EBUSY;
@@ -275,7 +274,9 @@ EXPORT_SYMBOL_GPL(driver_unregister);
* Call kset_find_obj() to iterate over list of drivers on
* a bus to find driver by name. Return driver if found.
*
- * Note that kset_find_obj increments driver's reference count.
+ * This routine provides no locking to prevent the driver it returns
+ * from being unregistered or unloaded while the caller is using it.
+ * The caller is responsible for preventing this.
*/
struct device_driver *driver_find(const char *name, struct bus_type
*bus)
{
@@ -283,6 +284,8 @@ struct device_driver *driver_find(const
struct driver_private *priv;

if (k) {
+ /* Drop reference added by kset_find_obj() */
+ kobject_put(k);
priv = to_driver(k);
return priv->driver;
}
Index: usb-3.3/drivers/input/gameport/gameport.c
usb-3.3.orig/drivers/input/gameport/gameport.c


+++ usb-3.3/drivers/input/gameport/gameport.c
@@ -449,7 +449,6 @@ static ssize_t gameport_rebind_driver(st
} else if ((drv = driver_find(buf, &gameport_bus)) != NULL) {
gameport_disconnect_port(gameport);
error = gameport_bind_driver(gameport, to_gameport_driver(drv));
- put_driver(drv);
} else {
error = -EINVAL;
}
Index: usb-3.3/drivers/input/serio/serio.c
usb-3.3.orig/drivers/input/serio/serio.c


+++ usb-3.3/drivers/input/serio/serio.c
@@ -441,7 +441,6 @@ static ssize_t serio_rebind_driver(struc
} else if ((drv = driver_find(buf, &serio_bus)) != NULL) {
serio_disconnect_port(serio);
error = serio_bind_driver(serio, to_serio_driver(drv));
- put_driver(drv);
serio_remove_duplicate_events(serio, SERIO_RESCAN_PORT);
} else {
error = -EINVAL;
Index: usb-3.3/drivers/media/video/cx18/cx18-alsa-main.c
usb-3.3.orig/drivers/media/video/cx18/cx18-alsa-main.c


+++ usb-3.3/drivers/media/video/cx18/cx18-alsa-main.c
@@ -285,7 +285,6 @@ static void __exit cx18_alsa_exit(void)

drv = driver_find("cx18", &pci_bus_type);
ret = driver_for_each_device(drv, NULL, NULL,
cx18_alsa_exit_callback);
- put_driver(drv);

cx18_ext_init = NULL;
printk(KERN_INFO "cx18-alsa: module unload complete");
Index: usb-3.3/drivers/media/video/ivtv/ivtvfb.c
usb-3.3.orig/drivers/media/video/ivtv/ivtvfb.c


+++ usb-3.3/drivers/media/video/ivtv/ivtvfb.c
@@ -1293,7 +1293,6 @@ static int __init ivtvfb_init(void)

drv = driver_find("ivtv", &pci_bus_type);
err = driver_for_each_device(drv, NULL, &registered,
ivtvfb_callback_init);
- put_driver(drv);
if (!registered) {
printk(KERN_ERR "ivtvfb: no cards found");
return -ENODEV;
@@ -1310,7 +1309,6 @@ static void ivtvfb_cleanup(void)

drv = driver_find("ivtv", &pci_bus_type);
err = driver_for_each_device(drv, NULL, NULL,
ivtvfb_callback_cleanup);
- put_driver(drv);
}

module_init(ivtvfb_init);
Index: usb-3.3/drivers/media/video/s5p-fimc/fimc-mdevice.c
usb-3.3.orig/drivers/media/video/s5p-fimc/fimc-mdevice.c


+++ usb-3.3/drivers/media/video/s5p-fimc/fimc-mdevice.c
@@ -344,16 +344,13 @@ static int fimc_md_register_platform_ent
return -ENODEV;
ret = driver_for_each_device(driver, NULL, fmd,
fimc_register_callback);
- put_driver(driver);
if (ret)
return ret;

driver = driver_find(CSIS_DRIVER_NAME, &platform_bus_type);
- if (driver) {
+ if (driver)
ret = driver_for_each_device(driver, NULL, fmd,
csis_register_callback);
- put_driver(driver);
- }
return ret;
}

Index: usb-3.3/drivers/media/video/s5p-tv/mixer_video.c
usb-3.3.orig/drivers/media/video/s5p-tv/mixer_video.c


+++ usb-3.3/drivers/media/video/s5p-tv/mixer_video.c
@@ -58,7 +58,6 @@ static struct v4l2_subdev *find_and_regi
}

done:
- put_driver(drv);
return sd;
}

Index: usb-3.3/drivers/s390/net/smsgiucv_app.c
usb-3.3.orig/drivers/s390/net/smsgiucv_app.c


+++ usb-3.3/drivers/s390/net/smsgiucv_app.c
@@ -168,7 +168,7 @@ static int __init smsgiucv_app_init(void
rc = dev_set_name(smsg_app_dev, KMSG_COMPONENT);
if (rc) {
kfree(smsg_app_dev);
- goto fail_put_driver;
+ goto fail;
}
smsg_app_dev->bus = &iucv_bus;
smsg_app_dev->parent = iucv_root;
@@ -177,7 +177,7 @@ static int __init smsgiucv_app_init(void
rc = device_register(smsg_app_dev);
if (rc) {
put_device(smsg_app_dev);
- goto fail_put_driver;
+ goto fail;
}

/* convert sender to uppercase characters */
@@ -191,12 +191,11 @@ static int __init smsgiucv_app_init(void
rc = smsg_register_callback(SMSG_PREFIX, smsg_app_callback);
if (rc) {
device_unregister(smsg_app_dev);
- goto fail_put_driver;
+ goto fail;
}

rc = 0;
-fail_put_driver:
- put_driver(smsgiucv_drv);
+fail:
return rc;
}
module_init(smsgiucv_app_init);




For the cx18-alsa and ivtvfb changes:

Acked-by: Andy Walls

These modules simply add an additional interface for userspace to access the functions provided by the video cards. Users of these particular modules know to unload them first, before unloading the cx18 or ivtv module.

Regards,
Andy
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Similar topics