Messages in this thread | | | Date | Mon, 18 Jul 2022 12:44:45 +0100 | Subject | Re: [PATCH -next,v2] nvmem: core: Fix memleak in nvmem_register() | From | Srinivas Kandagatla <> |
| |
On 16/07/2022 10:52, Gaosheng Cui wrote: > dev_set_name will alloc memory for nvmem->dev.kobj.name in > nvmem_register, when nvmem_validate_keepouts failed, nvmem's > memory will be freed and return, but nobody will free memory > for nvmem->dev.kobj.name, there will be memleak, so moving > dev_set_name after nvmem_validate_keepouts to fix it. > > Fixes: de0534df9347 ("nvmem: core: fix error handling while validating keepout regions") > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > drivers/nvmem/core.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index 1e3c754efd0d..a5d75edf509d 100644 > --- a/drivers/nvmem/core.c > +++ b/drivers/nvmem/core.c > @@ -808,20 +808,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > else if (!config->no_of_node) > nvmem->dev.of_node = config->dev->of_node; > > - switch (config->id) { > - case NVMEM_DEVID_NONE: > - dev_set_name(&nvmem->dev, "%s", config->name); > - break; > - case NVMEM_DEVID_AUTO: > - dev_set_name(&nvmem->dev, "%s%d", config->name, nvmem->id); > - break; > - default: > - dev_set_name(&nvmem->dev, "%s%d", > - config->name ? : "nvmem", > - config->name ? config->id : nvmem->id); > - break; > - } > - > nvmem->read_only = device_property_present(config->dev, "read-only") || > config->read_only || !nvmem->reg_write; > > @@ -838,6 +824,20 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > } > } > > + switch (config->id) { > + case NVMEM_DEVID_NONE: > + dev_set_name(&nvmem->dev, "%s", config->name); > + break; > + case NVMEM_DEVID_AUTO: > + dev_set_name(&nvmem->dev, "%s%d", config->name, nvmem->id); > + break; > + default: > + dev_set_name(&nvmem->dev, "%s%d", > + config->name ? : "nvmem", > + config->name ? config->id : nvmem->id); > + break; > + } > + The issue with this approach is that dev_err messages in nvmem_validate_keepouts() will show up without device name setup.
I think better option is to move nvmem_validate_keepouts() after device_register() and let the device core deal with cleaning name in error cases.
--srini
> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); > > rval = device_register(&nvmem->dev);
| |