* [PATCH v2 1/2] iio: buffer: Fix double-free in iio_buffers_alloc_sysfs_and_mask()
2021-10-13 9:49 [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization Andy Shevchenko
@ 2021-10-13 9:49 ` Andy Shevchenko
2021-10-13 9:49 ` [PATCH v2 2/2] iio: buffer: Use dedicated variable " Andy Shevchenko
2021-10-17 14:26 ` [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-10-13 9:49 UTC (permalink / raw)
To: Jonathan Cameron, Alexandru Ardelean, linux-iio, linux-kernel
Cc: Jonathan Cameron, Lars-Peter Clausen, Yang Yingliang, Hulk Robot,
Alexandru Ardelean, Andy Shevchenko
From: Yang Yingliang <yangyingliang@huawei.com>
When __iio_buffer_alloc_sysfs_and_mask() failed, 'unwind_idx' should be
set to 'i - 1' to prevent double-free when cleanup resources.
BUG: KASAN: double-free or invalid-free in __iio_buffer_free_sysfs_and_mask+0x32/0xb0 [industrialio]
Call Trace:
kfree+0x117/0x4c0
__iio_buffer_free_sysfs_and_mask+0x32/0xb0 [industrialio]
iio_buffers_alloc_sysfs_and_mask+0x60d/0x1570 [industrialio]
__iio_device_register+0x483/0x1a30 [industrialio]
ina2xx_probe+0x625/0x980 [ina2xx_adc]
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: ee708e6baacd ("iio: buffer: introduce support for attaching more IIO buffers")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/iio/industrialio-buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 4209e933ab80..bb181d11573c 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1616,7 +1616,7 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
buffer = iio_dev_opaque->attached_buffers[i];
ret = __iio_buffer_alloc_sysfs_and_mask(buffer, indio_dev, i);
if (ret) {
- unwind_idx = i;
+ unwind_idx = i - 1;
goto error_unwind_sysfs_and_mask;
}
}
--
2.33.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/2] iio: buffer: Use dedicated variable in iio_buffers_alloc_sysfs_and_mask()
2021-10-13 9:49 [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization Andy Shevchenko
2021-10-13 9:49 ` [PATCH v2 1/2] iio: buffer: Fix double-free in iio_buffers_alloc_sysfs_and_mask() Andy Shevchenko
@ 2021-10-13 9:49 ` Andy Shevchenko
2021-10-17 14:26 ` [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-10-13 9:49 UTC (permalink / raw)
To: Jonathan Cameron, Alexandru Ardelean, linux-iio, linux-kernel
Cc: Jonathan Cameron, Lars-Peter Clausen, Yang Yingliang, Andy Shevchenko
Use dedicated variable for index in the loop in the
iio_buffers_alloc_sysfs_and_mask(). This will make code cleaner and
less error prone as proved by previous changes done in this function.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/iio/industrialio-buffer.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index bb181d11573c..d53f8e6d5935 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1596,8 +1596,7 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
const struct iio_chan_spec *channels;
struct iio_buffer *buffer;
- int unwind_idx;
- int ret, i;
+ int ret, i, idx;
size_t sz;
channels = indio_dev->channels;
@@ -1612,15 +1611,12 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
if (!iio_dev_opaque->attached_buffers_cnt)
return 0;
- for (i = 0; i < iio_dev_opaque->attached_buffers_cnt; i++) {
- buffer = iio_dev_opaque->attached_buffers[i];
- ret = __iio_buffer_alloc_sysfs_and_mask(buffer, indio_dev, i);
- if (ret) {
- unwind_idx = i - 1;
+ for (idx = 0; idx < iio_dev_opaque->attached_buffers_cnt; idx++) {
+ buffer = iio_dev_opaque->attached_buffers[idx];
+ ret = __iio_buffer_alloc_sysfs_and_mask(buffer, indio_dev, idx);
+ if (ret)
goto error_unwind_sysfs_and_mask;
- }
}
- unwind_idx = iio_dev_opaque->attached_buffers_cnt - 1;
sz = sizeof(*(iio_dev_opaque->buffer_ioctl_handler));
iio_dev_opaque->buffer_ioctl_handler = kzalloc(sz, GFP_KERNEL);
@@ -1636,8 +1632,8 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
return 0;
error_unwind_sysfs_and_mask:
- for (; unwind_idx >= 0; unwind_idx--) {
- buffer = iio_dev_opaque->attached_buffers[unwind_idx];
+ while (idx--) {
+ buffer = iio_dev_opaque->attached_buffers[idx];
__iio_buffer_free_sysfs_and_mask(buffer);
}
return ret;
--
2.33.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization
2021-10-13 9:49 [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization Andy Shevchenko
2021-10-13 9:49 ` [PATCH v2 1/2] iio: buffer: Fix double-free in iio_buffers_alloc_sysfs_and_mask() Andy Shevchenko
2021-10-13 9:49 ` [PATCH v2 2/2] iio: buffer: Use dedicated variable " Andy Shevchenko
@ 2021-10-17 14:26 ` Jonathan Cameron
2021-11-15 11:12 ` Andy Shevchenko
2 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2021-10-17 14:26 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Alexandru Ardelean, linux-iio, linux-kernel,
Lars-Peter Clausen, Yang Yingliang
On Wed, 13 Oct 2021 12:49:21 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Yang submitted a fix, but I think the code can be refactored a bit to be more
> robust against similar mistakes in the future, if any.
>
> In v2:
> - put SoB Yang's patch (it's good for backporting)
> - added refactoring patch on top of Yang's fix
>
> Andy Shevchenko (1):
> iio: buffer: Use dedicated variable in
> iio_buffers_alloc_sysfs_and_mask()
>
> Yang Yingliang (1):
> iio: buffer: Fix double-free in iio_buffers_alloc_sysfs_and_mask()
>
> drivers/iio/industrialio-buffer.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
1st patch applied to the fixes-togreg branch of iio.git. I may well end up
sending these in the merge window anyway in which case I'll probably stick patch 2
on top of it before sending. If not I'll pick that up next cycle now.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization
2021-10-17 14:26 ` [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization Jonathan Cameron
@ 2021-11-15 11:12 ` Andy Shevchenko
2021-11-20 14:17 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2021-11-15 11:12 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, Alexandru Ardelean, linux-iio, linux-kernel,
Lars-Peter Clausen, Yang Yingliang
On Sun, Oct 17, 2021 at 03:26:11PM +0100, Jonathan Cameron wrote:
> On Wed, 13 Oct 2021 12:49:21 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > Yang submitted a fix, but I think the code can be refactored a bit to be more
> > robust against similar mistakes in the future, if any.
> >
> > In v2:
> > - put SoB Yang's patch (it's good for backporting)
> > - added refactoring patch on top of Yang's fix
> >
> > Andy Shevchenko (1):
> > iio: buffer: Use dedicated variable in
> > iio_buffers_alloc_sysfs_and_mask()
> >
> > Yang Yingliang (1):
> > iio: buffer: Fix double-free in iio_buffers_alloc_sysfs_and_mask()
> >
> > drivers/iio/industrialio-buffer.c | 18 +++++++-----------
> > 1 file changed, 7 insertions(+), 11 deletions(-)
> >
> 1st patch applied to the fixes-togreg branch of iio.git. I may well end up
> sending these in the merge window anyway in which case I'll probably stick patch 2
> on top of it before sending. If not I'll pick that up next cycle now.
Is it a right time now?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] iio: buffer: allocation and freeing buffers fix and optimization
2021-11-15 11:12 ` Andy Shevchenko
@ 2021-11-20 14:17 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-11-20 14:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Alexandru Ardelean, linux-iio, linux-kernel,
Lars-Peter Clausen, Yang Yingliang
On Mon, 15 Nov 2021 13:12:24 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Sun, Oct 17, 2021 at 03:26:11PM +0100, Jonathan Cameron wrote:
> > On Wed, 13 Oct 2021 12:49:21 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > Yang submitted a fix, but I think the code can be refactored a bit to be more
> > > robust against similar mistakes in the future, if any.
> > >
> > > In v2:
> > > - put SoB Yang's patch (it's good for backporting)
> > > - added refactoring patch on top of Yang's fix
> > >
> > > Andy Shevchenko (1):
> > > iio: buffer: Use dedicated variable in
> > > iio_buffers_alloc_sysfs_and_mask()
> > >
> > > Yang Yingliang (1):
> > > iio: buffer: Fix double-free in iio_buffers_alloc_sysfs_and_mask()
> > >
> > > drivers/iio/industrialio-buffer.c | 18 +++++++-----------
> > > 1 file changed, 7 insertions(+), 11 deletions(-)
> > >
> > 1st patch applied to the fixes-togreg branch of iio.git. I may well end up
> > sending these in the merge window anyway in which case I'll probably stick patch 2
> > on top of it before sending. If not I'll pick that up next cycle now.
>
> Is it a right time now?
>
Applied, but needed a bit of hand tweaking as patches have crossed with this.
Pushed out as testing for 0-day to see if we missed anything.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread