uio: Fix uio driver to refcount device
PATCH v7 1/1 uio: Fix uio driver to refcount device
Protect uio driver from its owner being unplugged while there are open fds. Embed struct device in struct uio_device, use refcounting on device, free uio_device on release. info struct passed in uio_register_device can be freed on unregister, so null out the field in uio_unregister_device and check accesses.
--- drivers/uio/uio.c | 60 ++++++++++++++++++++++++++++++---------------- include/linux/uio_driver.h | 2 +- 2 files changed, 41 insertions(+), 21 deletions(-)
v7: use dev_set_name and fix error path in uio_register_device
v6: ... and put them in the right place
v5: add patch version changes
v4: info struct passed in uio_register_device can be freed on unregister, so null
out the field in uio_unregister_device and check accesses.
v3: Embed struct device in struct uio_device, use refcounting on device, free
uio_device on release.
v2: Add blank line to header
drivers/uio/uio.c +40 -20
@@ -270,7 +270,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)if (!map_found) { map_found =1 ; idev->map_dir =kobject_create_and_add ("maps" , - &idev->dev->kobj); + &idev->dev.kobj);if (!idev->map_dir) goto err_map; }
@@ -295,7 +295,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)if (!portio_found) { portio_found =1 ; idev->portio_dir =kobject_create_and_add ("portio" , - &idev->dev->kobj); + &idev->dev.kobj);if (!idev->portio_dir) goto err_portio; }
@@ -334,7 +334,7 @@ err_map_kobj:kobject_put (&map->kobj); }kobject_put (idev->map_dir); -dev_err (idev->dev,"error creating sysfs files (%d)\n" , ret); +dev_err (&idev->dev,"error creating sysfs files (%d)\n" , ret);return ret; }
@@ -371,7 +371,7 @@ static int uio_get_minor(struct uio_device *idev) idev->minor = retval; retval =0 ; }else if (retval == -ENOSPC ) { -dev_err (idev->dev,"too many uio devices\n" ); +dev_err (&idev->dev,"too many uio devices\n" ); retval = -EINVAL ; }mutex_unlock (&minor_lock);
@@ -434,9 +434,11 @@ static int uio_open(struct inode *inode, struct file *filep) goto out; } +get_device (&idev->dev); +if (!try_module_get (idev->owner)) { ret = -ENODEV ; - goto out; + goto err_module_get; } listener =kmalloc (sizeof (*listener),GFP_KERNEL );
@@ -462,6 +464,9 @@ err_infoopen: err_alloc_listener:module_put (idev->owner); +err_module_get: +put_device (&idev->dev); + out:return ret; }
@@ -480,11 +485,12 @@ static int uio_release(struct inode *inode, struct file *filep)struct uio_listener *listener = filep->private_data;struct uio_device *idev = listener->dev; -if (idev->info->release) +if (idev->info && idev->info->release) ret = idev->info->release (idev->info, inode);module_put (idev->owner);kfree (listener); +put_device (&idev->dev);return ret; }
@@ -493,7 +499,7 @@ static unsigned int uio_poll(struct file *filep, poll_table *wait)struct uio_listener *listener = filep->private_data;struct uio_device *idev = listener->dev; -if (!idev->info->irq) +if (!idev->info || !idev->info->irq)return -EIO ;poll_wait (filep, &idev->wait, wait);
@@ -511,7 +517,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf,ssize_t retval;s32 event_count; -if (!idev->info->irq) +if (!idev->info || !idev->info->irq)return -EIO ;if (count !=sizeof (s32))
@@ -559,7 +565,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf,ssize_t retval;s32 irq_on; -if (!idev->info->irq) +if (!idev->info || !idev->info->irq)return -EIO ;if (count !=sizeof (s32))
@@ -785,6 +791,13 @@ static void release_uio_class(void)uio_major_cleanup (); } +static void uio_device_release (struct device *dev) +{ +struct uio_device *idev =dev_get_drvdata (dev); + +kfree (idev); +} +/** * uio_register_device - register a new userspace IO device * @owner: module that creates the new device
@@ -805,7 +818,7 @@ int __uio_register_device(struct module *owner, info->uio_dev =NULL ; - idev =devm_kzalloc (parent,sizeof (*idev),GFP_KERNEL ); + idev =kzalloc (sizeof (*idev),GFP_KERNEL );if (!idev) {return -ENOMEM ; }
@@ -819,14 +832,19 @@ int __uio_register_device(struct module *owner,if (ret)return ret; - idev->dev =device_create (&uio_class, parent, -MKDEV (uio_major, idev->minor), idev, -"uio%d" , idev->minor); -if (IS_ERR (idev->dev)) { -printk (KERN_ERR "UIO: device register failed\n" ); - ret =PTR_ERR (idev->dev); + idev->dev.devt =MKDEV (uio_major, idev->minor); + idev->dev.class = &uio_class; + idev->dev.parent = parent; + idev->dev.release = uio_device_release; +dev_set_drvdata (&idev->dev, idev); + + ret =dev_set_name (&idev->dev,"uio%d" , idev->minor); +if (ret) + goto err_device_create; + + ret =device_register (&idev->dev); +if (ret) goto err_device_create; - } ret =uio_dev_add_attributes (idev);if (ret)
@@ -835,7 +853,7 @@ int __uio_register_device(struct module *owner, info->uio_dev = idev;if (info->irq && (info->irq !=UIO_IRQ_CUSTOM )) { - ret =devm_request_irq (idev->dev, info->irq, uio_interrupt, + ret =request_irq (info->irq, uio_interrupt, info->irq_flags, info->name, idev);if (ret) goto err_request_irq;
@@ -846,7 +864,7 @@ int __uio_register_device(struct module *owner, err_request_irq:uio_dev_del_attributes (idev); err_uio_dev_add_attributes: -device_destroy (&uio_class,MKDEV (uio_major, idev->minor)); +device_unregister (&idev->dev); err_device_create:uio_free_minor (idev);return ret;
@@ -871,8 +889,10 @@ void uio_unregister_device(struct uio_info *info)uio_dev_del_attributes (idev); -device_destroy (&uio_class,MKDEV (uio_major, idev->minor)); +device_unregister (&idev->dev); +free_irq (idev->info->irq, idev); + idev->info =NULL ;return ; }EXPORT_SYMBOL_GPL (uio_unregister_device);
include/linux/uio_driver.h +1 -1
@@ -65,7 +65,7 @@ struct uio_port {struct uio_device {struct module *owner; -struct device *dev; +struct device dev;int minor;atomic_t event;struct fasync_struct *async_queue;
Signature · 1 line
2.1.4
Greg Kroah-Hartman
maintainerQuoted 4 lines · Brian Russell
Protect uio driver from its owner being unplugged while there are open fds. Embed struct device in struct
uio_device, use refcounting on device, freeuio_deviceon release. info struct passed inuio_register_devicecan be freed on unregister, so null out the field inuio_unregister_deviceand check accesses.
Minor nit, changelogs need to be linewrapped at 72 columns, if your editor is sane, it should have warned you about this :)
thanks,
greg k-h