* [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
* Re: [PATCH] iio: buffer: fix use-after-free for attached_buffers array
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:54 ` [PATCH v2] " Alexandru Ardelean
1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2021-03-07 12:36 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, Lars-Peter Clausen
On Sat, 6 Mar 2021 18:47:10 +0200
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> 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.
It feels a bit wrong to do direct freeing of stuff in a _put() call
given that kind of implies nothing will happen without some reference
count dropping to 0. We could think about renaming the function to
something like
iio_buffers_put_and_free_array() but is a bit long winded.
Otherwise, I'm fine with this but want to let it sit on list a tiny bit
longer before I take it as it's not totally trivial unlike the previous
one.
Jonathan
> 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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: buffer: fix use-after-free for attached_buffers array
2021-03-07 12:36 ` Jonathan Cameron
@ 2021-03-07 12:54 ` Lars-Peter Clausen
2021-03-07 18:35 ` Alexandru Ardelean
0 siblings, 1 reply; 7+ messages in thread
From: Lars-Peter Clausen @ 2021-03-07 12:54 UTC (permalink / raw)
To: Jonathan Cameron, Alexandru Ardelean; +Cc: linux-iio, linux-kernel
On 3/7/21 1:36 PM, Jonathan Cameron wrote:
> On Sat, 6 Mar 2021 18:47:10 +0200
> Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
>
>> 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.
> It feels a bit wrong to do direct freeing of stuff in a _put() call
> given that kind of implies nothing will happen without some reference
> count dropping to 0. We could think about renaming the function to
> something like
>
> iio_buffers_put_and_free_array() but is a bit long winded.
>
> Otherwise, I'm fine with this but want to let it sit on list a tiny bit
> longer before I take it as it's not totally trivial unlike the previous
> one.
Maybe to go with naming schema of iio_device_attach_buffer() call this
function iio_device_detach_buffers(). We grab the reference in attach,
and drop it in detach.
- Lars
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: buffer: fix use-after-free for attached_buffers array
2021-03-07 12:54 ` Lars-Peter Clausen
@ 2021-03-07 18:35 ` Alexandru Ardelean
0 siblings, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2021-03-07 18:35 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, LKML
On Sun, Mar 7, 2021 at 2:54 PM Lars-Peter Clausen <lars@metafoo.de> wrote:
>
> On 3/7/21 1:36 PM, Jonathan Cameron wrote:
> > On Sat, 6 Mar 2021 18:47:10 +0200
> > Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> >
> >> 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.
> > It feels a bit wrong to do direct freeing of stuff in a _put() call
> > given that kind of implies nothing will happen without some reference
> > count dropping to 0. We could think about renaming the function to
> > something like
> >
> > iio_buffers_put_and_free_array() but is a bit long winded.
> >
> > Otherwise, I'm fine with this but want to let it sit on list a tiny bit
> > longer before I take it as it's not totally trivial unlike the previous
> > one.
>
> Maybe to go with naming schema of iio_device_attach_buffer() call this
> function iio_device_detach_buffers(). We grab the reference in attach,
> and drop it in detach.
That actually sounds like it fits beautifully ( the
iio_device_detach_buffers() name ).
Thanks for the hint.
I'll send a V2.
I didn't consider more on the renaming of iio_buffers_put() because I
was a bit stressed by the silliness of the use-after-free bug.
Thanks
Alex
>
> - Lars
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] iio: buffer: fix use-after-free for attached_buffers array
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 18:54 ` Alexandru Ardelean
2021-03-08 7:29 ` Alexandru Ardelean
1 sibling, 1 reply; 7+ messages in thread
From: Alexandru Ardelean @ 2021-03-07 18:54 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().
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: 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/iio_core.h | 4 ++--
drivers/iio/industrialio-buffer.c | 9 +++++----
drivers/iio/industrialio-core.c | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 062fe16c6c49..8f4a9b264962 100644
--- 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 1a415e97174e..1ff7f731b044 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -326,7 +326,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;
@@ -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_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 d74fbac908df..3be5f75c2846 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1587,7 +1587,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);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] iio: buffer: fix use-after-free for attached_buffers array
2021-03-07 18:54 ` [PATCH v2] " Alexandru Ardelean
@ 2021-03-08 7:29 ` Alexandru Ardelean
2021-03-13 18:28 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Alexandru Ardelean @ 2021-03-08 7:29 UTC (permalink / raw)
To: linux-iio, LKML; +Cc: Jonathan Cameron, Lars-Peter Clausen
On Sun, Mar 7, 2021 at 8:55 PM Alexandru Ardelean
<ardeleanalex@gmail.com> wrote:
>
> 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: 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>
> ---
Forgot the changelog.
Changelog v1 -> v2:
* rename iio_buffers_put() -> iio_device_detach_buffers()
> drivers/iio/iio_core.h | 4 ++--
> drivers/iio/industrialio-buffer.c | 9 +++++----
> drivers/iio/industrialio-core.c | 2 +-
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index 062fe16c6c49..8f4a9b264962 100644
> --- 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 1a415e97174e..1ff7f731b044 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -326,7 +326,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;
> @@ -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_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 d74fbac908df..3be5f75c2846 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1587,7 +1587,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);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] iio: buffer: fix use-after-free for attached_buffers array
2021-03-08 7:29 ` Alexandru Ardelean
@ 2021-03-13 18:28 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2021-03-13 18:28 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, LKML, Lars-Peter Clausen
On Mon, 8 Mar 2021 09:29:39 +0200
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> On Sun, Mar 7, 2021 at 8:55 PM Alexandru Ardelean
> <ardeleanalex@gmail.com> wrote:
> >
> > 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: 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>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to poke at it.
Thanks,
Jonathan
> > ---
>
> Forgot the changelog.
>
> Changelog v1 -> v2:
> * rename iio_buffers_put() -> iio_device_detach_buffers()
>
> > drivers/iio/iio_core.h | 4 ++--
> > drivers/iio/industrialio-buffer.c | 9 +++++----
> > drivers/iio/industrialio-core.c | 2 +-
> > 3 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> > index 062fe16c6c49..8f4a9b264962 100644
> > --- 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 1a415e97174e..1ff7f731b044 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -326,7 +326,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;
> > @@ -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_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 d74fbac908df..3be5f75c2846 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -1587,7 +1587,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);
> > --
> > 2.25.1
> >
^ permalink raw reply [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).