forked from mirrors/linux
		
	uio: Prevent device destruction while fds are open
Prevent destruction of a uio_device while user space apps hold open file descriptors to that device. Further, access to the 'info' member of the struct uio_device is protected by spinlock. This is to ensure stale pointers to data not under control of the UIO subsystem are not dereferenced. Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
		
							parent
							
								
									81daa406c2
								
							
						
					
					
						commit
						a93e7b3315
					
				
					 2 changed files with 75 additions and 27 deletions
				
			
		|  | @ -270,7 +270,7 @@ static int uio_dev_add_attributes(struct uio_device *idev) | ||||||
| 		if (!map_found) { | 		if (!map_found) { | ||||||
| 			map_found = 1; | 			map_found = 1; | ||||||
| 			idev->map_dir = kobject_create_and_add("maps", | 			idev->map_dir = kobject_create_and_add("maps", | ||||||
| 							&idev->dev->kobj); | 							&idev->dev.kobj); | ||||||
| 			if (!idev->map_dir) { | 			if (!idev->map_dir) { | ||||||
| 				ret = -ENOMEM; | 				ret = -ENOMEM; | ||||||
| 				goto err_map; | 				goto err_map; | ||||||
|  | @ -299,7 +299,7 @@ static int uio_dev_add_attributes(struct uio_device *idev) | ||||||
| 		if (!portio_found) { | 		if (!portio_found) { | ||||||
| 			portio_found = 1; | 			portio_found = 1; | ||||||
| 			idev->portio_dir = kobject_create_and_add("portio", | 			idev->portio_dir = kobject_create_and_add("portio", | ||||||
| 							&idev->dev->kobj); | 							&idev->dev.kobj); | ||||||
| 			if (!idev->portio_dir) { | 			if (!idev->portio_dir) { | ||||||
| 				ret = -ENOMEM; | 				ret = -ENOMEM; | ||||||
| 				goto err_portio; | 				goto err_portio; | ||||||
|  | @ -342,7 +342,7 @@ static int uio_dev_add_attributes(struct uio_device *idev) | ||||||
| 		kobject_put(&map->kobj); | 		kobject_put(&map->kobj); | ||||||
| 	} | 	} | ||||||
| 	kobject_put(idev->map_dir); | 	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; | 	return ret; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -379,7 +379,7 @@ static int uio_get_minor(struct uio_device *idev) | ||||||
| 		idev->minor = retval; | 		idev->minor = retval; | ||||||
| 		retval = 0; | 		retval = 0; | ||||||
| 	} else if (retval == -ENOSPC) { | 	} else if (retval == -ENOSPC) { | ||||||
| 		dev_err(idev->dev, "too many uio devices\n"); | 		dev_err(&idev->dev, "too many uio devices\n"); | ||||||
| 		retval = -EINVAL; | 		retval = -EINVAL; | ||||||
| 	} | 	} | ||||||
| 	mutex_unlock(&minor_lock); | 	mutex_unlock(&minor_lock); | ||||||
|  | @ -433,6 +433,7 @@ static int uio_open(struct inode *inode, struct file *filep) | ||||||
| 	struct uio_device *idev; | 	struct uio_device *idev; | ||||||
| 	struct uio_listener *listener; | 	struct uio_listener *listener; | ||||||
| 	int ret = 0; | 	int ret = 0; | ||||||
|  | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	mutex_lock(&minor_lock); | 	mutex_lock(&minor_lock); | ||||||
| 	idev = idr_find(&uio_idr, iminor(inode)); | 	idev = idr_find(&uio_idr, iminor(inode)); | ||||||
|  | @ -442,9 +443,11 @@ static int uio_open(struct inode *inode, struct file *filep) | ||||||
| 		goto out; | 		goto out; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	get_device(&idev->dev); | ||||||
|  | 
 | ||||||
| 	if (!try_module_get(idev->owner)) { | 	if (!try_module_get(idev->owner)) { | ||||||
| 		ret = -ENODEV; | 		ret = -ENODEV; | ||||||
| 		goto out; | 		goto err_module_get; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	listener = kmalloc(sizeof(*listener), GFP_KERNEL); | 	listener = kmalloc(sizeof(*listener), GFP_KERNEL); | ||||||
|  | @ -457,11 +460,13 @@ static int uio_open(struct inode *inode, struct file *filep) | ||||||
| 	listener->event_count = atomic_read(&idev->event); | 	listener->event_count = atomic_read(&idev->event); | ||||||
| 	filep->private_data = listener; | 	filep->private_data = listener; | ||||||
| 
 | 
 | ||||||
| 	if (idev->info->open) { | 	spin_lock_irqsave(&idev->info_lock, flags); | ||||||
|  | 	if (idev->info && idev->info->open) | ||||||
| 		ret = idev->info->open(idev->info, inode); | 		ret = idev->info->open(idev->info, inode); | ||||||
| 		if (ret) | 	spin_unlock_irqrestore(&idev->info_lock, flags); | ||||||
| 			goto err_infoopen; | 	if (ret) | ||||||
| 	} | 		goto err_infoopen; | ||||||
|  | 
 | ||||||
| 	return 0; | 	return 0; | ||||||
| 
 | 
 | ||||||
| err_infoopen: | err_infoopen: | ||||||
|  | @ -470,6 +475,9 @@ static int uio_open(struct inode *inode, struct file *filep) | ||||||
| err_alloc_listener: | err_alloc_listener: | ||||||
| 	module_put(idev->owner); | 	module_put(idev->owner); | ||||||
| 
 | 
 | ||||||
|  | err_module_get: | ||||||
|  | 	put_device(&idev->dev); | ||||||
|  | 
 | ||||||
| out: | out: | ||||||
| 	return ret; | 	return ret; | ||||||
| } | } | ||||||
|  | @ -487,12 +495,16 @@ static int uio_release(struct inode *inode, struct file *filep) | ||||||
| 	int ret = 0; | 	int ret = 0; | ||||||
| 	struct uio_listener *listener = filep->private_data; | 	struct uio_listener *listener = filep->private_data; | ||||||
| 	struct uio_device *idev = listener->dev; | 	struct uio_device *idev = listener->dev; | ||||||
|  | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	if (idev->info->release) | 	spin_lock_irqsave(&idev->info_lock, flags); | ||||||
|  | 	if (idev->info && idev->info->release) | ||||||
| 		ret = idev->info->release(idev->info, inode); | 		ret = idev->info->release(idev->info, inode); | ||||||
|  | 	spin_unlock_irqrestore(&idev->info_lock, flags); | ||||||
| 
 | 
 | ||||||
| 	module_put(idev->owner); | 	module_put(idev->owner); | ||||||
| 	kfree(listener); | 	kfree(listener); | ||||||
|  | 	put_device(&idev->dev); | ||||||
| 	return ret; | 	return ret; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -500,9 +512,16 @@ static __poll_t uio_poll(struct file *filep, poll_table *wait) | ||||||
| { | { | ||||||
| 	struct uio_listener *listener = filep->private_data; | 	struct uio_listener *listener = filep->private_data; | ||||||
| 	struct uio_device *idev = listener->dev; | 	struct uio_device *idev = listener->dev; | ||||||
|  | 	__poll_t ret = 0; | ||||||
|  | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	if (!idev->info->irq) | 	spin_lock_irqsave(&idev->info_lock, flags); | ||||||
| 		return -EIO; | 	if (!idev->info || !idev->info->irq) | ||||||
|  | 		ret = -EIO; | ||||||
|  | 	spin_unlock_irqrestore(&idev->info_lock, flags); | ||||||
|  | 
 | ||||||
|  | 	if (ret) | ||||||
|  | 		return ret; | ||||||
| 
 | 
 | ||||||
| 	poll_wait(filep, &idev->wait, wait); | 	poll_wait(filep, &idev->wait, wait); | ||||||
| 	if (listener->event_count != atomic_read(&idev->event)) | 	if (listener->event_count != atomic_read(&idev->event)) | ||||||
|  | @ -516,11 +535,17 @@ static ssize_t uio_read(struct file *filep, char __user *buf, | ||||||
| 	struct uio_listener *listener = filep->private_data; | 	struct uio_listener *listener = filep->private_data; | ||||||
| 	struct uio_device *idev = listener->dev; | 	struct uio_device *idev = listener->dev; | ||||||
| 	DECLARE_WAITQUEUE(wait, current); | 	DECLARE_WAITQUEUE(wait, current); | ||||||
| 	ssize_t retval; | 	ssize_t retval = 0; | ||||||
| 	s32 event_count; | 	s32 event_count; | ||||||
|  | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	if (!idev->info->irq) | 	spin_lock_irqsave(&idev->info_lock, flags); | ||||||
| 		return -EIO; | 	if (!idev->info || !idev->info->irq) | ||||||
|  | 		retval = -EIO; | ||||||
|  | 	spin_unlock_irqrestore(&idev->info_lock, flags); | ||||||
|  | 
 | ||||||
|  | 	if (retval) | ||||||
|  | 		return retval; | ||||||
| 
 | 
 | ||||||
| 	if (count != sizeof(s32)) | 	if (count != sizeof(s32)) | ||||||
| 		return -EINVAL; | 		return -EINVAL; | ||||||
|  | @ -567,8 +592,10 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, | ||||||
| 	struct uio_device *idev = listener->dev; | 	struct uio_device *idev = listener->dev; | ||||||
| 	ssize_t retval; | 	ssize_t retval; | ||||||
| 	s32 irq_on; | 	s32 irq_on; | ||||||
|  | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	if (!idev->info->irq) { | 	spin_lock_irqsave(&idev->info_lock, flags); | ||||||
|  | 	if (!idev->info || !idev->info->irq) { | ||||||
| 		retval = -EIO; | 		retval = -EIO; | ||||||
| 		goto out; | 		goto out; | ||||||
| 	} | 	} | ||||||
|  | @ -591,6 +618,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, | ||||||
| 	retval = idev->info->irqcontrol(idev->info, irq_on); | 	retval = idev->info->irqcontrol(idev->info, irq_on); | ||||||
| 
 | 
 | ||||||
| out: | out: | ||||||
|  | 	spin_unlock_irqrestore(&idev->info_lock, flags); | ||||||
| 	return retval ? retval : sizeof(s32); | 	return retval ? retval : sizeof(s32); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -803,6 +831,13 @@ static void release_uio_class(void) | ||||||
| 	uio_major_cleanup(); | 	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 |  * uio_register_device - register a new userspace IO device | ||||||
|  * @owner:	module that creates the new device |  * @owner:	module that creates the new device | ||||||
|  | @ -823,13 +858,14 @@ int __uio_register_device(struct module *owner, | ||||||
| 
 | 
 | ||||||
| 	info->uio_dev = NULL; | 	info->uio_dev = NULL; | ||||||
| 
 | 
 | ||||||
| 	idev = devm_kzalloc(parent, sizeof(*idev), GFP_KERNEL); | 	idev = kzalloc(sizeof(*idev), GFP_KERNEL); | ||||||
| 	if (!idev) { | 	if (!idev) { | ||||||
| 		return -ENOMEM; | 		return -ENOMEM; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	idev->owner = owner; | 	idev->owner = owner; | ||||||
| 	idev->info = info; | 	idev->info = info; | ||||||
|  | 	spin_lock_init(&idev->info_lock); | ||||||
| 	init_waitqueue_head(&idev->wait); | 	init_waitqueue_head(&idev->wait); | ||||||
| 	atomic_set(&idev->event, 0); | 	atomic_set(&idev->event, 0); | ||||||
| 
 | 
 | ||||||
|  | @ -837,14 +873,19 @@ int __uio_register_device(struct module *owner, | ||||||
| 	if (ret) | 	if (ret) | ||||||
| 		return ret; | 		return ret; | ||||||
| 
 | 
 | ||||||
| 	idev->dev = device_create(&uio_class, parent, | 	idev->dev.devt = MKDEV(uio_major, idev->minor); | ||||||
| 				  MKDEV(uio_major, idev->minor), idev, | 	idev->dev.class = &uio_class; | ||||||
| 				  "uio%d", idev->minor); | 	idev->dev.parent = parent; | ||||||
| 	if (IS_ERR(idev->dev)) { | 	idev->dev.release = uio_device_release; | ||||||
| 		printk(KERN_ERR "UIO: device register failed\n"); | 	dev_set_drvdata(&idev->dev, idev); | ||||||
| 		ret = PTR_ERR(idev->dev); | 
 | ||||||
|  | 	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; | 		goto err_device_create; | ||||||
| 	} |  | ||||||
| 
 | 
 | ||||||
| 	ret = uio_dev_add_attributes(idev); | 	ret = uio_dev_add_attributes(idev); | ||||||
| 	if (ret) | 	if (ret) | ||||||
|  | @ -872,7 +913,7 @@ int __uio_register_device(struct module *owner, | ||||||
| err_request_irq: | err_request_irq: | ||||||
| 	uio_dev_del_attributes(idev); | 	uio_dev_del_attributes(idev); | ||||||
| err_uio_dev_add_attributes: | err_uio_dev_add_attributes: | ||||||
| 	device_destroy(&uio_class, MKDEV(uio_major, idev->minor)); | 	device_unregister(&idev->dev); | ||||||
| err_device_create: | err_device_create: | ||||||
| 	uio_free_minor(idev); | 	uio_free_minor(idev); | ||||||
| 	return ret; | 	return ret; | ||||||
|  | @ -887,6 +928,7 @@ EXPORT_SYMBOL_GPL(__uio_register_device); | ||||||
| void uio_unregister_device(struct uio_info *info) | void uio_unregister_device(struct uio_info *info) | ||||||
| { | { | ||||||
| 	struct uio_device *idev; | 	struct uio_device *idev; | ||||||
|  | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	if (!info || !info->uio_dev) | 	if (!info || !info->uio_dev) | ||||||
| 		return; | 		return; | ||||||
|  | @ -900,7 +942,11 @@ void uio_unregister_device(struct uio_info *info) | ||||||
| 	if (info->irq && info->irq != UIO_IRQ_CUSTOM) | 	if (info->irq && info->irq != UIO_IRQ_CUSTOM) | ||||||
| 		free_irq(info->irq, idev); | 		free_irq(info->irq, idev); | ||||||
| 
 | 
 | ||||||
| 	device_destroy(&uio_class, MKDEV(uio_major, idev->minor)); | 	spin_lock_irqsave(&idev->info_lock, flags); | ||||||
|  | 	idev->info = NULL; | ||||||
|  | 	spin_unlock_irqrestore(&idev->info_lock, flags); | ||||||
|  | 
 | ||||||
|  | 	device_unregister(&idev->dev); | ||||||
| 
 | 
 | ||||||
| 	return; | 	return; | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -14,6 +14,7 @@ | ||||||
| #ifndef _UIO_DRIVER_H_ | #ifndef _UIO_DRIVER_H_ | ||||||
| #define _UIO_DRIVER_H_ | #define _UIO_DRIVER_H_ | ||||||
| 
 | 
 | ||||||
|  | #include <linux/device.h> | ||||||
| #include <linux/fs.h> | #include <linux/fs.h> | ||||||
| #include <linux/interrupt.h> | #include <linux/interrupt.h> | ||||||
| 
 | 
 | ||||||
|  | @ -68,12 +69,13 @@ struct uio_port { | ||||||
| 
 | 
 | ||||||
| struct uio_device { | struct uio_device { | ||||||
|         struct module           *owner; |         struct module           *owner; | ||||||
|         struct device           *dev; | 	struct device		dev; | ||||||
|         int                     minor; |         int                     minor; | ||||||
|         atomic_t                event; |         atomic_t                event; | ||||||
|         struct fasync_struct    *async_queue; |         struct fasync_struct    *async_queue; | ||||||
|         wait_queue_head_t       wait; |         wait_queue_head_t       wait; | ||||||
|         struct uio_info         *info; |         struct uio_info         *info; | ||||||
|  | 	spinlock_t		info_lock; | ||||||
|         struct kobject          *map_dir; |         struct kobject          *map_dir; | ||||||
|         struct kobject          *portio_dir; |         struct kobject          *portio_dir; | ||||||
| }; | }; | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Hamish Martin
						Hamish Martin