Commit e407fd655bf9b40c38cba29aa7d38149989798bb

Authored by Lars-Peter Clausen
Committed by Greg Kroah-Hartman
1 parent 04723de09d

iio: Fix potential use after free

There is no guarantee that the last reference to the iio device has already been
dropped when iio_device_free is called. This means that we can up calling
iio_dev_release after iio_device_free which will lead to a use after free. As
the general rule the struct containing the device should always be freed in the
release callback.

This is what this patch does, it moves freeing the iio device struct as well as
releasing the idr reference to the release callback. To ensure that the device
is not freed before calling iio_device_free the device_unregister call in
iio_device_unregister is broken apart. iio_device_unregister will now only call
device_del to remove the device from the system and iio_device_free will call
put_device to drop the reference we obtained in iio_devce_alloc.

We also have to take care that calling iio_device_free without having called
iio_device_register still works (i.e. this can happen if something failed during
device initialization). For this to work properly two minor changes were
necessary: channel_attr_list needs to be initialized in iio_device_alloc and we
have to check whether the chrdev has been registered before releasing it in
iio_device_release.

This change also brings iio_device_unregister and iio_device_free more in sync
with iio_device_register and iio_device_alloc which call device_add and
device_initialize respectively.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Showing 1 changed file with 9 additions and 7 deletions Side-by-side Diff

drivers/iio/industrialio-core.c
... ... @@ -661,7 +661,6 @@
661 661 * New channel registration method - relies on the fact a group does
662 662 * not need to be initialized if it is name is NULL.
663 663 */
664   - INIT_LIST_HEAD(&indio_dev->channel_attr_list);
665 664 if (indio_dev->channels)
666 665 for (i = 0; i < indio_dev->num_channels; i++) {
667 666 ret = iio_device_add_channel_sysfs(indio_dev,
668 667  
... ... @@ -725,12 +724,16 @@
725 724 static void iio_dev_release(struct device *device)
726 725 {
727 726 struct iio_dev *indio_dev = dev_to_iio_dev(device);
728   - cdev_del(&indio_dev->chrdev);
  727 + if (indio_dev->chrdev.dev)
  728 + cdev_del(&indio_dev->chrdev);
729 729 if (indio_dev->modes & INDIO_BUFFER_TRIGGERED)
730 730 iio_device_unregister_trigger_consumer(indio_dev);
731 731 iio_device_unregister_eventset(indio_dev);
732 732 iio_device_unregister_sysfs(indio_dev);
733 733 iio_device_unregister_debugfs(indio_dev);
  734 +
  735 + ida_simple_remove(&iio_ida, indio_dev->id);
  736 + kfree(indio_dev);
734 737 }
735 738  
736 739 static struct device_type iio_dev_type = {
... ... @@ -761,6 +764,7 @@
761 764 dev_set_drvdata(&dev->dev, (void *)dev);
762 765 mutex_init(&dev->mlock);
763 766 mutex_init(&dev->info_exist_lock);
  767 + INIT_LIST_HEAD(&dev->channel_attr_list);
764 768  
765 769 dev->id = ida_simple_get(&iio_ida, 0, 0, GFP_KERNEL);
766 770 if (dev->id < 0) {
... ... @@ -778,10 +782,8 @@
778 782  
779 783 void iio_device_free(struct iio_dev *dev)
780 784 {
781   - if (dev) {
782   - ida_simple_remove(&iio_ida, dev->id);
783   - kfree(dev);
784   - }
  785 + if (dev)
  786 + put_device(&dev->dev);
785 787 }
786 788 EXPORT_SYMBOL(iio_device_free);
787 789  
... ... @@ -902,7 +904,7 @@
902 904 mutex_lock(&indio_dev->info_exist_lock);
903 905 indio_dev->info = NULL;
904 906 mutex_unlock(&indio_dev->info_exist_lock);
905   - device_unregister(&indio_dev->dev);
  907 + device_del(&indio_dev->dev);
906 908 }
907 909 EXPORT_SYMBOL(iio_device_unregister);
908 910 subsys_initcall(iio_init);