2023-07-18 14:27:14

by Daniel Golle

[permalink] [raw]
Subject: [PATCH 1/2] nvmem: core: clear sysfs attributes for each NVMEM device

Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
having any cells in order to make sure sysfs attributes of a previously
registered NVMEM device are not accidentally reused for a follow-up
device which doesn't have any cells.

Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")
Signed-off-by: Daniel Golle <[email protected]>
---
drivers/nvmem/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 6c04a9cf6919f..70e951088826d 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -458,9 +458,10 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)

mutex_lock(&nvmem_mutex);

- if (list_empty(&nvmem->cells))
+ if (list_empty(&nvmem->cells)) {
+ nvmem_cells_group.bin_attrs = NULL;
goto unlock_mutex;
-
+ }
/* Allocate an array of attributes with a sentinel */
ncells = list_count_nodes(&nvmem->cells);
cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,
--
2.41.0



2023-07-18 15:06:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] nvmem: core: clear sysfs attributes for each NVMEM device

On Tue, Jul 18, 2023 at 02:55:31PM +0100, Daniel Golle wrote:
> Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> having any cells in order to make sure sysfs attributes of a previously
> registered NVMEM device are not accidentally reused for a follow-up
> device which doesn't have any cells.
>
> Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")

Where is this git commit id? I don't see it in Linus's tree.

> Signed-off-by: Daniel Golle <[email protected]>
> ---
> drivers/nvmem/core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)

Your patches were not threaded/attached to each other either, so our
tools can't take them together. Can you fix that up and use 'git
send-email' or other such tools to have this show up properly when you
send a v2?

thanks,

greg k-h

2023-07-18 15:08:10

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] nvmem: core: clear sysfs attributes for each NVMEM device

On Tue, Jul 18, 2023 at 02:55:31PM +0100, Daniel Golle wrote:
> Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> having any cells in order to make sure sysfs attributes of a previously
> registered NVMEM device are not accidentally reused for a follow-up
> device which doesn't have any cells.

Wait, attributes and devices should NEVER be reused, how is that
happening here?

And just setting the attribute field to NULL doesn't free or clean up
anything, right? Did memory just leak with this?

confused,

greg k-h

2023-07-18 15:42:19

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH 1/2] nvmem: core: clear sysfs attributes for each NVMEM device

HI Daniel,

On 18/07/2023 14:55, Daniel Golle wrote:
> Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> having any cells in order to make sure sysfs attributes of a previously
> registered NVMEM device are not accidentally reused for a follow-up
> device which doesn't have any cells.
>
> Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")

These patches are dropped out of nvmem next branch as it was breaking
some existing users.


--srini
> Signed-off-by: Daniel Golle <[email protected]>
> ---
> drivers/nvmem/core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 6c04a9cf6919f..70e951088826d 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -458,9 +458,10 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
>
> mutex_lock(&nvmem_mutex);
>
> - if (list_empty(&nvmem->cells))
> + if (list_empty(&nvmem->cells)) {
> + nvmem_cells_group.bin_attrs = NULL;
> goto unlock_mutex;
> -
> + }
> /* Allocate an array of attributes with a sentinel */
> ncells = list_count_nodes(&nvmem->cells);
> cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,

2023-07-18 15:50:18

by Daniel Golle

[permalink] [raw]
Subject: Re: [PATCH 1/2] nvmem: core: clear sysfs attributes for each NVMEM device

On Tue, Jul 18, 2023 at 03:55:56PM +0100, Srinivas Kandagatla wrote:
> HI Daniel,
>
> On 18/07/2023 14:55, Daniel Golle wrote:
> > Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> > having any cells in order to make sure sysfs attributes of a previously
> > registered NVMEM device are not accidentally reused for a follow-up
> > device which doesn't have any cells.
> >
> > Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")
>
> These patches are dropped out of nvmem next branch as it was breaking some
> existing users.

Ok. I've encountered those commits in linux-next and can confirm that
they were definitely also breaking things here, hence my patches at
least partially fixing that.

I agree that reverting them for now and reworking them seems to be the
better option in this case, hence my patches won't be needed as such.

>
>
> --srini
> > Signed-off-by: Daniel Golle <[email protected]>
> > ---
> > drivers/nvmem/core.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 6c04a9cf6919f..70e951088826d 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -458,9 +458,10 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
> > mutex_lock(&nvmem_mutex);
> > - if (list_empty(&nvmem->cells))
> > + if (list_empty(&nvmem->cells)) {
> > + nvmem_cells_group.bin_attrs = NULL;
> > goto unlock_mutex;
> > -
> > + }
> > /* Allocate an array of attributes with a sentinel */
> > ncells = list_count_nodes(&nvmem->cells);
> > cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,

2023-07-19 08:23:14

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 1/2] nvmem: core: clear sysfs attributes for each NVMEM device

Hi Daniel,

[email protected] wrote on Tue, 18 Jul 2023 16:29:07 +0100:

> On Tue, Jul 18, 2023 at 03:55:56PM +0100, Srinivas Kandagatla wrote:
> > HI Daniel,
> >
> > On 18/07/2023 14:55, Daniel Golle wrote:
> > > Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> > > having any cells in order to make sure sysfs attributes of a previously
> > > registered NVMEM device are not accidentally reused for a follow-up
> > > device which doesn't have any cells.
> > >
> > > Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")
> >
> > These patches are dropped out of nvmem next branch as it was breaking some
> > existing users.
>
> Ok. I've encountered those commits in linux-next and can confirm that
> they were definitely also breaking things here, hence my patches at
> least partially fixing that.
>
> I agree that reverting them for now and reworking them seems to be the
> better option in this case, hence my patches won't be needed as such.

Quite the opposite, on my setup I don't have any breakage but as you
already fixed the cell name issue (also reported by Chen-yu), if you
agree, I will include these fixes (or an improved version) in my next
proposal.

Thanks,
Miquèl