Re: [PATCH v2] nvmem: core: fix possibly memleak when use nvmem_cell_info_to_nvmem_cell()

From: Srinivas Kandagatla
Date: Wed Sep 23 2020 - 12:25:10 EST




On 23/09/2020 17:23, Vadym Kochan wrote:
On Wed, Sep 23, 2020 at 04:51:06PM +0100, Srinivas Kandagatla wrote:


On 23/09/2020 15:51, Vadym Kochan wrote:
- return nvmem_cell_write(&cell, buf, cell.bytes);
+ rc = nvmem_cell_write(&cell, buf, cell.bytes);
+ if (rc)
+ kfree_const(cell->name);
+
+ return rc;
}
EXPORT_SYMBOL_GPL(nvmem_device_cell_write);
------------------------>cut<---------------------------

--srini

But is it really needed to kstrdup(cell->name) for nvmem_device_cell_{read,write} ?
This boils down to if we want to use same api to parse nvmem_cell_info or
not!

If we want to keep this simple, we can either explicitly add free for
successful caller to nvmem_cell_info_to_nvmem_cell()!

Or

use something like what you did, but new api needs more clarity!
May be renaming __nvmem_cell_info_to_nvmem_cell to
nvmem_cell_info_to_nvmem_cell_no_alloc would clarify that a bit!


Naming is most difficult thing, what about __nvmem_cell_info_to_nvmem_cell_{unsafe,nodup}() ?
At least this is an indication to be carefully here.

nvmem_cell_info_to_nvmem_cell_nodup() should be good!

--srini

Also can you make sure that linewrapping on function names be inline with
existing code.

Please send v3 with that changes!


--srini
It is used only for log error in case the unaligned access did not
pass the check