linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: buffer: fix use-after-free for attached_buffers array
@ 2021-03-06 16:47 Alexandru Ardelean
  2021-03-07 12:36 ` Jonathan Cameron
  2021-03-07 18:54 ` [PATCH v2] " Alexandru Ardelean
  0 siblings, 2 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2021-03-06 16:47 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean, Lars-Peter Clausen

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().

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: 36f3118c414d ("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>
---
 drivers/iio/industrialio-buffer.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 1a415e97174e..3d0712651d43 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -336,6 +336,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,
@@ -1764,7 +1766,6 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 		buffer = iio_dev_opaque->attached_buffers[unwind_idx];
 		__iio_buffer_free_sysfs_and_mask(buffer);
 	}
-	kfree(iio_dev_opaque->attached_buffers);
 	return ret;
 }
 
@@ -1786,8 +1787,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);
 }
 
 /**
@@ -2062,6 +2061,8 @@ static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma)
  * 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_buffers_put() call,
+ * which is handled by the iio_device_free().
  */
 int iio_device_attach_buffer(struct iio_dev *indio_dev,
 			     struct iio_buffer *buffer)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-13 18:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-06 16:47 [PATCH] iio: buffer: fix use-after-free for attached_buffers array Alexandru Ardelean
2021-03-07 12:36 ` Jonathan Cameron
2021-03-07 12:54   ` Lars-Peter Clausen
2021-03-07 18:35     ` Alexandru Ardelean
2021-03-07 18:54 ` [PATCH v2] " Alexandru Ardelean
2021-03-08  7:29   ` Alexandru Ardelean
2021-03-13 18:28     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).