2022-08-16 14:00:53

by Gaosheng Cui

[permalink] [raw]
Subject: [PATCH -next 0/2] A few fixup patches for nvmem

This series contains a fixup patches to fix possible memleak,
and add error handling of dev_set_name to keep nvmem code
implementation consistent, in addition, it can also avoid
the possibility illegal address access. Thanks!

Gaosheng Cui (2):
nvmem: core: Fix memleak in nvmem_register()
nvmem: core: add error handling for dev_set_name

drivers/nvmem/core.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)

--
2.25.1


2022-08-16 14:01:07

by Gaosheng Cui

[permalink] [raw]
Subject: [PATCH -next 1/2] nvmem: core: Fix memleak in nvmem_register()

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
nvmem_validate_keepouts() after device_register() and let
the device core deal with cleaning name in error cases.

Fixes: de0534df9347 ("nvmem: core: fix error handling while validating keepout regions")
Signed-off-by: Gaosheng Cui <[email protected]>
---
drivers/nvmem/core.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 1e3c754efd0d..2164efd12ba9 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -829,21 +829,18 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
nvmem->dev.groups = nvmem_dev_groups;
#endif

- if (nvmem->nkeepout) {
- rval = nvmem_validate_keepouts(nvmem);
- if (rval) {
- ida_free(&nvmem_ida, nvmem->id);
- kfree(nvmem);
- return ERR_PTR(rval);
- }
- }
-
dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);

rval = device_register(&nvmem->dev);
if (rval)
goto err_put_device;

+ if (nvmem->nkeepout) {
+ rval = nvmem_validate_keepouts(nvmem);
+ if (rval)
+ goto err_device_del;
+ }
+
if (config->compat) {
rval = nvmem_sysfs_setup_compat(nvmem, config);
if (rval)
--
2.25.1

2022-08-16 14:39:45

by Gaosheng Cui

[permalink] [raw]
Subject: [PATCH -next 2/2] nvmem: core: add error handling for dev_set_name

The type of return value of dev_set_name is int, which may return
wrong result, so we add error handling for it to reclaim memory
of nvmem resource, and return early when an error occurs.

Signed-off-by: Gaosheng Cui <[email protected]>
---
drivers/nvmem/core.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 2164efd12ba9..321d7d63e068 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -810,18 +810,24 @@ 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);
+ rval = dev_set_name(&nvmem->dev, "%s", config->name);
break;
case NVMEM_DEVID_AUTO:
- dev_set_name(&nvmem->dev, "%s%d", config->name, nvmem->id);
+ rval = dev_set_name(&nvmem->dev, "%s%d", config->name, nvmem->id);
break;
default:
- dev_set_name(&nvmem->dev, "%s%d",
+ rval = dev_set_name(&nvmem->dev, "%s%d",
config->name ? : "nvmem",
config->name ? config->id : nvmem->id);
break;
}

+ if (rval) {
+ ida_free(&nvmem_ida, nvmem->id);
+ kfree(nvmem);
+ return ERR_PTR(rval);
+ }
+
nvmem->read_only = device_property_present(config->dev, "read-only") ||
config->read_only || !nvmem->reg_write;

--
2.25.1

2022-08-17 09:09:02

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH -next 0/2] A few fixup patches for nvmem



On 16/08/2022 14:49, Gaosheng Cui wrote:
> This series contains a fixup patches to fix possible memleak,
> and add error handling of dev_set_name to keep nvmem code
> implementation consistent, in addition, it can also avoid
> the possibility illegal address access. Thanks!
>
> Gaosheng Cui (2):
> nvmem: core: Fix memleak in nvmem_register()
> nvmem: core: add error handling for dev_set_name
>

I have applied these patches for now but remember the subject line does
not reflect the patch version or the cover letter does not include any
changes done over the versions.

Please take a look at
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html


thanks,
Srini

> drivers/nvmem/core.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>

2022-08-17 09:42:15

by Gaosheng Cui

[permalink] [raw]
Subject: Re: [PATCH -next 0/2] A few fixup patches for nvmem

I'll take a closer look at this link, thank you very much!

Gaosheng

在 2022/8/17 16:53, Srinivas Kandagatla 写道:
>
>
> On 16/08/2022 14:49, Gaosheng Cui wrote:
>> This series contains a fixup patches to fix possible memleak,
>> and add error handling of dev_set_name to keep nvmem code
>> implementation consistent, in addition, it can also avoid
>> the possibility illegal address access. Thanks!
>>
>> Gaosheng Cui (2):
>>    nvmem: core: Fix memleak in nvmem_register()
>>    nvmem: core: add error handling for dev_set_name
>>
>
> I have applied these patches for now but remember the subject line
> does not reflect the patch version or the cover letter does not
> include any changes done over the versions.
>
> Please take a look at
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
>
>
> thanks,
> Srini
>
>>   drivers/nvmem/core.c | 27 +++++++++++++++------------
>>   1 file changed, 15 insertions(+), 12 deletions(-)
>>
> .