Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Alexandru Ardelean and committed by
Jonathan Cameron
218bc53d 4f2d9cce

+8 -7
+2 -2
drivers/iio/iio_core.h
··· 77 77 78 78 void iio_disable_all_buffers(struct iio_dev *indio_dev); 79 79 void iio_buffer_wakeup_poll(struct iio_dev *indio_dev); 80 - void iio_buffers_put(struct iio_dev *indio_dev); 80 + void iio_device_detach_buffers(struct iio_dev *indio_dev); 81 81 82 82 #else 83 83 ··· 93 93 94 94 static inline void iio_disable_all_buffers(struct iio_dev *indio_dev) {} 95 95 static inline void iio_buffer_wakeup_poll(struct iio_dev *indio_dev) {} 96 - static inline void iio_buffers_put(struct iio_dev *indio_dev) {} 96 + static inline void iio_device_detach_buffers(struct iio_dev *indio_dev) {} 97 97 98 98 #endif 99 99
+5 -4
drivers/iio/industrialio-buffer.c
··· 242 242 } 243 243 EXPORT_SYMBOL(iio_buffer_init); 244 244 245 - void iio_buffers_put(struct iio_dev *indio_dev) 245 + void iio_device_detach_buffers(struct iio_dev *indio_dev) 246 246 { 247 247 struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); 248 248 struct iio_buffer *buffer; ··· 252 252 buffer = iio_dev_opaque->attached_buffers[i]; 253 253 iio_buffer_put(buffer); 254 254 } 255 + 256 + kfree(iio_dev_opaque->attached_buffers); 255 257 } 256 258 257 259 static ssize_t iio_show_scan_index(struct device *dev, ··· 1635 1633 buffer = iio_dev_opaque->attached_buffers[unwind_idx]; 1636 1634 __iio_buffer_free_sysfs_and_mask(buffer); 1637 1635 } 1638 - kfree(iio_dev_opaque->attached_buffers); 1639 1636 return ret; 1640 1637 } 1641 1638 ··· 1656 1655 buffer = iio_dev_opaque->attached_buffers[i]; 1657 1656 __iio_buffer_free_sysfs_and_mask(buffer); 1658 1657 } 1659 - 1660 - kfree(iio_dev_opaque->attached_buffers); 1661 1658 } 1662 1659 1663 1660 /** ··· 1778 1779 * This function attaches a buffer to a IIO device. The buffer stays attached to 1779 1780 * the device until the device is freed. For legacy reasons, the first attached 1780 1781 * buffer will also be assigned to 'indio_dev->buffer'. 1782 + * The array allocated here, will be free'd via the iio_device_detach_buffers() 1783 + * call which is handled by the iio_device_free(). 1781 1784 */ 1782 1785 int iio_device_attach_buffer(struct iio_dev *indio_dev, 1783 1786 struct iio_buffer *buffer)
+1 -1
drivers/iio/industrialio-core.c
··· 1586 1586 iio_device_unregister_eventset(indio_dev); 1587 1587 iio_device_unregister_sysfs(indio_dev); 1588 1588 1589 - iio_buffers_put(indio_dev); 1589 + iio_device_detach_buffers(indio_dev); 1590 1590 1591 1591 ida_simple_remove(&iio_ida, indio_dev->id); 1592 1592 kfree(iio_dev_opaque);