| Message ID | 20221123180151.2160033-11-michael@walle.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvmem: core: introduce NVMEM layouts | expand |
Hi Michael,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Michael-Walle/nvmem-core-introduce-NVMEM-layouts/20221124-020554
patch link: https://lore.kernel.org/r/20221123180151.2160033-11-michael%40walle.cc
patch subject: [PATCH v4 10/20] nvmem: core: use nvmem_add_one_cell() in nvmem_add_cells_from_of()
config: i386-randconfig-m021
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
New smatch warnings:
drivers/nvmem/core.c:731 nvmem_add_cells_from_of() warn: possible memory leak of 'cell'
Old smatch warnings:
drivers/nvmem/core.c:735 nvmem_add_cells_from_of() warn: possible memory leak of 'cell'
vim +/cell +731 drivers/nvmem/core.c
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 689 static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 690 {
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 691 struct device *dev = &nvmem->dev;
7ae6478b304bc0 Srinivas Kandagatla 2021-10-13 692 struct nvmem_cell_entry *cell;
18f50dbcfd3676 Michael Walle 2022-11-23 693 struct device_node *child;
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 694 const __be32 *addr;
18f50dbcfd3676 Michael Walle 2022-11-23 695 int len, ret;
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 696
18f50dbcfd3676 Michael Walle 2022-11-23 697 for_each_child_of_node(dev->of_node, child) {
18f50dbcfd3676 Michael Walle 2022-11-23 698 struct nvmem_cell_info info = {0};
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 699
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 700 addr = of_get_property(child, "reg", &len);
0445efacec75b8 Ahmad Fatoum 2021-01-29 701 if (!addr)
0445efacec75b8 Ahmad Fatoum 2021-01-29 702 continue;
0445efacec75b8 Ahmad Fatoum 2021-01-29 703 if (len < 2 * sizeof(u32)) {
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 704 dev_err(dev, "nvmem: invalid reg on %pOF\n", child);
63879e2964bcee Christophe JAILLET 2021-06-11 705 of_node_put(child);
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 706 return -EINVAL;
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 707 }
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 708
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 709 cell = kzalloc(sizeof(*cell), GFP_KERNEL);
63879e2964bcee Christophe JAILLET 2021-06-11 710 if (!cell) {
63879e2964bcee Christophe JAILLET 2021-06-11 711 of_node_put(child);
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 712 return -ENOMEM;
63879e2964bcee Christophe JAILLET 2021-06-11 713 }
Seems like "cell" is not used any more so this just leaks.
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 714
18f50dbcfd3676 Michael Walle 2022-11-23 715 info.offset = be32_to_cpup(addr++);
18f50dbcfd3676 Michael Walle 2022-11-23 716 info.bytes = be32_to_cpup(addr);
18f50dbcfd3676 Michael Walle 2022-11-23 717 info.name = kasprintf(GFP_KERNEL, "%pOFn", child);
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 718
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 719 addr = of_get_property(child, "bits", &len);
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 720 if (addr && len == (2 * sizeof(u32))) {
18f50dbcfd3676 Michael Walle 2022-11-23 721 info.bit_offset = be32_to_cpup(addr++);
18f50dbcfd3676 Michael Walle 2022-11-23 722 info.nbits = be32_to_cpup(addr);
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 723 }
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 724
18f50dbcfd3676 Michael Walle 2022-11-23 725 info.np = of_node_get(child);
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 726
18f50dbcfd3676 Michael Walle 2022-11-23 727 ret = nvmem_add_one_cell(nvmem, &info);
18f50dbcfd3676 Michael Walle 2022-11-23 728 kfree(info.name);
18f50dbcfd3676 Michael Walle 2022-11-23 729 if (ret) {
63879e2964bcee Christophe JAILLET 2021-06-11 730 of_node_put(child);
18f50dbcfd3676 Michael Walle 2022-11-23 @731 return ret;
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 732 }
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 733 }
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 734
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 735 return 0;
e888d445ac33a5 Bartosz Golaszewski 2018-09-21 736 }
Am 2022-12-03 09:30, schrieb Dan Carpenter: > Hi Michael, > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Michael-Walle/nvmem-core-introduce-NVMEM-layouts/20221124-020554 > patch link: > https://lore.kernel.org/r/20221123180151.2160033-11-michael%40walle.cc > patch subject: [PATCH v4 10/20] nvmem: core: use nvmem_add_one_cell() > in nvmem_add_cells_from_of() > config: i386-randconfig-m021 > compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <error27@gmail.com> > > New smatch warnings: > drivers/nvmem/core.c:731 nvmem_add_cells_from_of() warn: possible > memory leak of 'cell' > > Old smatch warnings: > drivers/nvmem/core.c:735 nvmem_add_cells_from_of() warn: possible > memory leak of 'cell' > > vim +/cell +731 drivers/nvmem/core.c > > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 689 static int > nvmem_add_cells_from_of(struct nvmem_device *nvmem) > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 690 { > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 691 struct device > *dev = &nvmem->dev; > 7ae6478b304bc0 Srinivas Kandagatla 2021-10-13 692 struct > nvmem_cell_entry *cell; > 18f50dbcfd3676 Michael Walle 2022-11-23 693 struct device_node > *child; > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 694 const __be32 > *addr; > 18f50dbcfd3676 Michael Walle 2022-11-23 695 int len, ret; > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 696 > 18f50dbcfd3676 Michael Walle 2022-11-23 697 > for_each_child_of_node(dev->of_node, child) { > 18f50dbcfd3676 Michael Walle 2022-11-23 698 struct > nvmem_cell_info info = {0}; > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 699 > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 700 addr = > of_get_property(child, "reg", &len); > 0445efacec75b8 Ahmad Fatoum 2021-01-29 701 if (!addr) > 0445efacec75b8 Ahmad Fatoum 2021-01-29 702 continue; > 0445efacec75b8 Ahmad Fatoum 2021-01-29 703 if (len < 2 * > sizeof(u32)) { > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 704 dev_err(dev, > "nvmem: invalid reg on %pOF\n", child); > 63879e2964bcee Christophe JAILLET 2021-06-11 705 > of_node_put(child); > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 706 return -EINVAL; > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 707 } > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 708 > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 709 cell = > kzalloc(sizeof(*cell), GFP_KERNEL); > 63879e2964bcee Christophe JAILLET 2021-06-11 710 if (!cell) { > 63879e2964bcee Christophe JAILLET 2021-06-11 711 > of_node_put(child); > e888d445ac33a5 Bartosz Golaszewski 2018-09-21 712 return -ENOMEM; > 63879e2964bcee Christophe JAILLET 2021-06-11 713 } > > Seems like "cell" is not used any more so this just leaks. Damn, what a stupid bug from me. Thanks for catching this. -michael
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index cb25bf29dea7..26459d582e99 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -688,15 +688,15 @@ static int nvmem_validate_keepouts(struct nvmem_device *nvmem) static int nvmem_add_cells_from_of(struct nvmem_device *nvmem) { - struct device_node *parent, *child; struct device *dev = &nvmem->dev; struct nvmem_cell_entry *cell; + struct device_node *child; const __be32 *addr; - int len; + int len, ret; - parent = dev->of_node; + for_each_child_of_node(dev->of_node, child) { + struct nvmem_cell_info info = {0}; - for_each_child_of_node(parent, child) { addr = of_get_property(child, "reg", &len); if (!addr) continue; @@ -712,34 +712,24 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem) return -ENOMEM; } - cell->nvmem = nvmem; - cell->offset = be32_to_cpup(addr++); - cell->bytes = be32_to_cpup(addr); - cell->name = kasprintf(GFP_KERNEL, "%pOFn", child); + info.offset = be32_to_cpup(addr++); + info.bytes = be32_to_cpup(addr); + info.name = kasprintf(GFP_KERNEL, "%pOFn", child); addr = of_get_property(child, "bits", &len); if (addr && len == (2 * sizeof(u32))) { - cell->bit_offset = be32_to_cpup(addr++); - cell->nbits = be32_to_cpup(addr); + info.bit_offset = be32_to_cpup(addr++); + info.nbits = be32_to_cpup(addr); } - if (cell->nbits) - cell->bytes = DIV_ROUND_UP( - cell->nbits + cell->bit_offset, - BITS_PER_BYTE); + info.np = of_node_get(child); - if (!IS_ALIGNED(cell->offset, nvmem->stride)) { - dev_err(dev, "cell %s unaligned to nvmem stride %d\n", - cell->name, nvmem->stride); - /* Cells already added will be freed later. */ - kfree_const(cell->name); - kfree(cell); + ret = nvmem_add_one_cell(nvmem, &info); + kfree(info.name); + if (ret) { of_node_put(child); - return -EINVAL; + return ret; } - - cell->np = of_node_get(child); - nvmem_cell_entry_add(cell); } return 0;
Convert nvmem_add_cells_from_of() to use the new nvmem_add_one_cell(). This will remove duplicate code and it will make it possible to add a hook to a nvmem layout in between, which can change fields before the cell is finally added. Signed-off-by: Michael Walle <michael@walle.cc> --- changes since v3: - none changes since v2: - none changes since v1: - new patch drivers/nvmem/core.c | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-)