Messages in this thread | | | Subject | Re: [PATCH -next,v2] nvmem: core: Fix memleak in nvmem_register() | From | cuigaosheng <> | Date | Mon, 18 Jul 2022 20:29:17 +0800 |
| |
Thanks for your ideas,
I have made another patch, moving nvmem_validate_keepouts() after device_register() and let the device core deal with cleaning name in error cases, it's a better option for the issue.
Thanks,
在 2022/7/18 19:44, 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); > .
| |