Messages in this thread | | | Subject | Re: [PATCH] nvmem: core: fix error path in nvmem_add_cells() | From | Srinivas Kandagatla <> | Date | Wed, 17 Feb 2016 10:48:54 +0000 |
| |
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 <linux@rasmusvillemoes.dk> > --- > 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
| |