Hi Rasmus,
Thanks for the patch,
On 08/02/16 21:04, Rasmus Villemoes wrote:
The current code fails to nvmem_cell_drop(cells[0]) - even worse, if
the loop above fails already at i==0, we'll enter an essentially
infinite loop doing nvmem_cell_drop on cells[-1], cells[-2], ... which
is unlikely to end well.
I agree, it would fail in case of zero.
Also, we're not freeing the temporary backing array cells on the error
path.
Signed-off-by: Rasmus Villemoes <[email protected]>
---
drivers/nvmem/core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 6fd4e5a5ef4a..1e65eccfea83 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -288,9 +288,11 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
return 0;
err:
- while (--i)
+ while (i--)
nvmem_cell_drop(cells[i]);
No, this will not work.
3 issues,
1> If we enter this err path from nvmem_cell_info_to_nvmem_cell()
failures, you would be accessing already freed cells[i].
2> accessing un-allocated cells[i].
3> you would be trying to drop cells which are not in the list.
This is what you need here to fix it correctly.
while (--i >= 0)
+ kfree(cells);
This change looks good.
+
return rval;
}
--srini
