iio: buffer: fix use-after-free for attached_buffers array

Thanks to Lars for finding this.
The free of the 'attached_buffers' array should be done as late as
possible. This change moves it to iio_buffers_put(), which looks like
the best place for it, since it takes place right before the IIO device
data is free'd.
The free of this array will be handled by calling iio_device_free().
The iio_buffers_put() function is renamed to iio_device_detach_buffers()
since the role of this function changes a bit.

It looks like this issue was ocurring on the error path of
iio_buffers_alloc_sysfs_and_mask() and in
iio_buffers_free_sysfs_and_mask()

Added a comment in the doc-header of iio_device_attach_buffer() to
mention how this will be free'd in case anyone is reading the code
and becoming confused about it.

Fixes: ee708e6baacd ("iio: buffer: introduce support for attaching more IIO buffers")
Reported-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Link: https://lore.kernel.org/r/20210307185444.32924-1-ardeleanalex@gmail.com
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 062fe16c6c49aeed8d5a2a6031ef16685d3ce4e1..8f4a9b264962e8580b04e785a484132ca70cfa77 100644 (file)
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -77,7 +77,7 @@ void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev);
 
 void iio_disable_all_buffers(struct iio_dev *indio_dev);
 void iio_buffer_wakeup_poll(struct iio_dev *indio_dev);
-void iio_buffers_put(struct iio_dev *indio_dev);
+void iio_device_detach_buffers(struct iio_dev *indio_dev);
 
 #else
 
@@ -93,7 +93,7 @@ static inline void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev) {}
 
 static inline void iio_disable_all_buffers(struct iio_dev *indio_dev) {}
 static inline void iio_buffer_wakeup_poll(struct iio_dev *indio_dev) {}
-static inline void iio_buffers_put(struct iio_dev *indio_dev) {}
+static inline void iio_device_detach_buffers(struct iio_dev *indio_dev) {}
 
 #endif
 

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index a48e494a9fbb31cfe39ce4adba6d4fab1d1780df..ee5aab9d4a233180205a7022a1ae0b3089b22331 100644 (file)
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -242,7 +242,7 @@ void iio_buffer_init(struct iio_buffer *buffer)
 }
 EXPORT_SYMBOL(iio_buffer_init);
 
-void iio_buffers_put(struct iio_dev *indio_dev)
+void iio_device_detach_buffers(struct iio_dev *indio_dev)
 {
        struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
        struct iio_buffer *buffer;
@@ -252,6 +252,8 @@ void iio_buffers_put(struct iio_dev *indio_dev)
                buffer = iio_dev_opaque->attached_buffers[i];
                iio_buffer_put(buffer);
        }
+
+       kfree(iio_dev_opaque->attached_buffers);
 }
 
 static ssize_t iio_show_scan_index(struct device *dev,
@@ -1633,7 +1635,6 @@ error_unwind_sysfs_and_mask:
                buffer = iio_dev_opaque->attached_buffers[unwind_idx];
                __iio_buffer_free_sysfs_and_mask(buffer);
        }
-       kfree(iio_dev_opaque->attached_buffers);
        return ret;
 }
 
@@ -1655,8 +1656,6 @@ void iio_buffers_free_sysfs_and_mask(struct iio_dev *indio_dev)
                buffer = iio_dev_opaque->attached_buffers[i];
                __iio_buffer_free_sysfs_and_mask(buffer);
        }
-
-       kfree(iio_dev_opaque->attached_buffers);
 }
 
 /**
@@ -1779,6 +1778,8 @@ EXPORT_SYMBOL_GPL(iio_buffer_put);
  * This function attaches a buffer to a IIO device. The buffer stays attached to
  * the device until the device is freed. For legacy reasons, the first attached
  * buffer will also be assigned to 'indio_dev->buffer'.
+ * The array allocated here, will be free'd via the iio_device_detach_buffers()
+ * call which is handled by the iio_device_free().
  */
 int iio_device_attach_buffer(struct iio_dev *indio_dev,
                             struct iio_buffer *buffer)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index cb2735d2ae4b18448616c1348cfc7bf3bae4771e..b5750edf935c9c253b102bb04f5967fec862433b 100644 (file)
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1586,7 +1586,7 @@ static void iio_dev_release(struct device *device)
        iio_device_unregister_eventset(indio_dev);
        iio_device_unregister_sysfs(indio_dev);
 
-       iio_buffers_put(indio_dev);
+       iio_device_detach_buffers(indio_dev);
 
        ida_simple_remove(&iio_ida, indio_dev->id);
        kfree(iio_dev_opaque);