* [PATCH] iio: core: fix memleak in iio_device_register_sysfs
@ 2023-12-08 7:31 Dinghao Liu
2023-12-10 13:32 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Dinghao Liu @ 2023-12-08 7:31 UTC (permalink / raw)
To: dinghao.liu
Cc: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean,
linux-iio, linux-kernel
When iio_device_register_sysfs_group() fails, we should
free iio_dev_opaque->chan_attr_group.attrs to prevent
potential memleak.
Fixes: 32f171724e5c ("iio: core: rework iio device group creation")
Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
---
drivers/iio/industrialio-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index c77745b594bd..e6d3d07a4c83 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1581,10 +1581,13 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
ret = iio_device_register_sysfs_group(indio_dev,
&iio_dev_opaque->chan_attr_group);
if (ret)
- goto error_clear_attrs;
+ goto error_free_chan_attrs;
return 0;
+error_free_chan_attrs:
+ kfree(iio_dev_opaque->chan_attr_group.attrs);
+ iio_dev_opaque->chan_attr_group.attrs = NULL;
error_clear_attrs:
iio_free_chan_devattr_list(&iio_dev_opaque->channel_attr_list);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: core: fix memleak in iio_device_register_sysfs
2023-12-08 7:31 [PATCH] iio: core: fix memleak in iio_device_register_sysfs Dinghao Liu
@ 2023-12-10 13:32 ` Jonathan Cameron
2023-12-17 13:28 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2023-12-10 13:32 UTC (permalink / raw)
To: Dinghao Liu
Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel
On Fri, 8 Dec 2023 15:31:19 +0800
Dinghao Liu <dinghao.liu@zju.edu.cn> wrote:
> When iio_device_register_sysfs_group() fails, we should
> free iio_dev_opaque->chan_attr_group.attrs to prevent
> potential memleak.
>
> Fixes: 32f171724e5c ("iio: core: rework iio device group creation")
> Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
Hi.
Looks good to me, but I'd like to leave this one on the list a little
longer to see if anyone else has comments.
Jonathan
> ---
> drivers/iio/industrialio-core.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index c77745b594bd..e6d3d07a4c83 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1581,10 +1581,13 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
> ret = iio_device_register_sysfs_group(indio_dev,
> &iio_dev_opaque->chan_attr_group);
> if (ret)
> - goto error_clear_attrs;
> + goto error_free_chan_attrs;
>
> return 0;
>
> +error_free_chan_attrs:
> + kfree(iio_dev_opaque->chan_attr_group.attrs);
> + iio_dev_opaque->chan_attr_group.attrs = NULL;
> error_clear_attrs:
> iio_free_chan_devattr_list(&iio_dev_opaque->channel_attr_list);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: core: fix memleak in iio_device_register_sysfs
2023-12-10 13:32 ` Jonathan Cameron
@ 2023-12-17 13:28 ` Jonathan Cameron
2024-02-11 19:16 ` andy.shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2023-12-17 13:28 UTC (permalink / raw)
To: Dinghao Liu
Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel
On Sun, 10 Dec 2023 13:32:28 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 8 Dec 2023 15:31:19 +0800
> Dinghao Liu <dinghao.liu@zju.edu.cn> wrote:
>
> > When iio_device_register_sysfs_group() fails, we should
> > free iio_dev_opaque->chan_attr_group.attrs to prevent
> > potential memleak.
> >
> > Fixes: 32f171724e5c ("iio: core: rework iio device group creation")
> > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> Hi.
>
> Looks good to me, but I'd like to leave this one on the list a little
> longer to see if anyone else has comments.
>
Guess no comments!
Applied to the fixes-togreg branch of iio.git. I might not get another
fixes request out far enough in advance of the merge window in which case
this will go in around then instead.
Also marked for stable
Jonathan
> Jonathan
>
> > ---
> > drivers/iio/industrialio-core.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index c77745b594bd..e6d3d07a4c83 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -1581,10 +1581,13 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
> > ret = iio_device_register_sysfs_group(indio_dev,
> > &iio_dev_opaque->chan_attr_group);
> > if (ret)
> > - goto error_clear_attrs;
> > + goto error_free_chan_attrs;
> >
> > return 0;
> >
> > +error_free_chan_attrs:
> > + kfree(iio_dev_opaque->chan_attr_group.attrs);
> > + iio_dev_opaque->chan_attr_group.attrs = NULL;
> > error_clear_attrs:
> > iio_free_chan_devattr_list(&iio_dev_opaque->channel_attr_list);
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: core: fix memleak in iio_device_register_sysfs
2023-12-17 13:28 ` Jonathan Cameron
@ 2024-02-11 19:16 ` andy.shevchenko
2024-02-11 19:37 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: andy.shevchenko @ 2024-02-11 19:16 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Dinghao Liu, Lars-Peter Clausen, Alexandru Ardelean, linux-iio,
linux-kernel
Sun, Dec 17, 2023 at 01:28:00PM +0000, Jonathan Cameron kirjoitti:
> On Sun, 10 Dec 2023 13:32:28 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Fri, 8 Dec 2023 15:31:19 +0800
> > Dinghao Liu <dinghao.liu@zju.edu.cn> wrote:
> >
> > > When iio_device_register_sysfs_group() fails, we should
> > > free iio_dev_opaque->chan_attr_group.attrs to prevent
> > > potential memleak.
> > >
> > > Fixes: 32f171724e5c ("iio: core: rework iio device group creation")
> > > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> > Hi.
> >
> > Looks good to me, but I'd like to leave this one on the list a little
> > longer to see if anyone else has comments.
> >
> Guess no comments!
This patch does not fix anything.
Yet, it might be considered as one that increases robustness, but with this applied the
goto
https://elixir.bootlin.com/linux/v6.8-rc3/source/drivers/iio/industrialio-core.c#L2007
can be amended, right?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: core: fix memleak in iio_device_register_sysfs
2024-02-11 19:16 ` andy.shevchenko
@ 2024-02-11 19:37 ` Jonathan Cameron
2024-02-11 20:19 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2024-02-11 19:37 UTC (permalink / raw)
To: andy.shevchenko
Cc: Dinghao Liu, Lars-Peter Clausen, Alexandru Ardelean, linux-iio,
linux-kernel
On Sun, 11 Feb 2024 21:16:32 +0200
andy.shevchenko@gmail.com wrote:
> Sun, Dec 17, 2023 at 01:28:00PM +0000, Jonathan Cameron kirjoitti:
> > On Sun, 10 Dec 2023 13:32:28 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > > On Fri, 8 Dec 2023 15:31:19 +0800
> > > Dinghao Liu <dinghao.liu@zju.edu.cn> wrote:
> > >
> > > > When iio_device_register_sysfs_group() fails, we should
> > > > free iio_dev_opaque->chan_attr_group.attrs to prevent
> > > > potential memleak.
> > > >
> > > > Fixes: 32f171724e5c ("iio: core: rework iio device group creation")
> > > > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> > > Hi.
> > >
> > > Looks good to me, but I'd like to leave this one on the list a little
> > > longer to see if anyone else has comments.
> > >
> > Guess no comments!
>
> This patch does not fix anything.
>
> Yet, it might be considered as one that increases robustness, but with this applied the
> goto
> https://elixir.bootlin.com/linux/v6.8-rc3/source/drivers/iio/industrialio-core.c#L2007
> can be amended, right?
>
I'm lost. That goto results in a call to
iio_buffers_free_sysfs_and_mask(indio_dev);
which continues to undo stuff from before that call.
Now if it did
iio_device_unregister_sysfs(indio_dev);
(as per the label above it in the cleanup) then I'd agree.
Perhaps I'm misreading the code flow here?
All this code is supposed to be side effect free on error so I'm
keen on the change even if there is another path where it gets cleaned
up that I'm missing.
Jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: core: fix memleak in iio_device_register_sysfs
2024-02-11 19:37 ` Jonathan Cameron
@ 2024-02-11 20:19 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2024-02-11 20:19 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Dinghao Liu, Lars-Peter Clausen, Alexandru Ardelean, linux-iio,
linux-kernel
On Sun, Feb 11, 2024 at 9:37 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Sun, 11 Feb 2024 21:16:32 +0200
> andy.shevchenko@gmail.com wrote:
>
> > Sun, Dec 17, 2023 at 01:28:00PM +0000, Jonathan Cameron kirjoitti:
> > > On Sun, 10 Dec 2023 13:32:28 +0000
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > > On Fri, 8 Dec 2023 15:31:19 +0800
> > > > Dinghao Liu <dinghao.liu@zju.edu.cn> wrote:
> > > >
> > > > > When iio_device_register_sysfs_group() fails, we should
> > > > > free iio_dev_opaque->chan_attr_group.attrs to prevent
> > > > > potential memleak.
> > > > >
> > > > > Fixes: 32f171724e5c ("iio: core: rework iio device group creation")
> > > > > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> > > > Hi.
> > > >
> > > > Looks good to me, but I'd like to leave this one on the list a little
> > > > longer to see if anyone else has comments.
> > > >
> > > Guess no comments!
> >
> > This patch does not fix anything.
> >
> > Yet, it might be considered as one that increases robustness, but with this applied the
> > goto
> > https://elixir.bootlin.com/linux/v6.8-rc3/source/drivers/iio/industrialio-core.c#L2007
> > can be amended, right?
> >
>
> I'm lost. That goto results in a call to
> iio_buffers_free_sysfs_and_mask(indio_dev);
> which continues to undo stuff from before that call.
> Now if it did
> iio_device_unregister_sysfs(indio_dev);
> (as per the label above it in the cleanup) then I'd agree.
Ah, it's me who hasn't noticed the word "buffer" in the goto label.
Yes, you are right!
> Perhaps I'm misreading the code flow here?
>
> All this code is supposed to be side effect free on error so I'm
> keen on the change even if there is another path where it gets cleaned
> up that I'm missing.
Everything is fine, sorry for the noise.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-11 20:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-08 7:31 [PATCH] iio: core: fix memleak in iio_device_register_sysfs Dinghao Liu
2023-12-10 13:32 ` Jonathan Cameron
2023-12-17 13:28 ` Jonathan Cameron
2024-02-11 19:16 ` andy.shevchenko
2024-02-11 19:37 ` Jonathan Cameron
2024-02-11 20:19 ` Andy Shevchenko
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).