[OpenWrt-Devel] [PATCH] uci: do not access invalid memory when updating an existing section

[Enrico Mioso]

Hello, and thank you for your help, and review.
I agree on not changing behaviour. the problem is not actually a memory leak, but the fact that ptr->s->options seems to not be a valid pointer at that point.
At least, this is what valgrind suggested, if I am not wrong or interpreting wrongly the output.

Thanks again for all,
Enrico
On Mon, Aug 13, 2018 at 09:07:32PM +0800, Yousong Zhou wrote:
> On Sun, 12 Aug 2018 at 14:41, Enrico Mioso <mrkiko.rs at gmail.com> wrote:
> >
> > If a new section with the same name and type of an old one is found, a
> > memory reallocation happens. Still, the options list for the section is
> > not reinitialized, hence a stale pointer is being used.
> >
> > Signed-off-by: Enrico Mioso <mrkiko.rs at gmail.com>
> > ---
> >  list.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/list.c b/list.c
> > index 25aec56..d934216 100644
> > --- a/list.c
> > +++ b/list.c
> > @@ -717,6 +717,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
> >                         ptr->last = uci_realloc(ctx, ptr->s, sizeof(struct uci_section));
> >                         ptr->s = uci_to_section(ptr->last);
> >                         uci_list_fixup(&ptr->s->e.list);
> > +                       uci_list_init(&ptr->s->options);
> >                 } else {
> >                         free(ptr->s->type);
> >                 }
> 
> If I understand the original code right, the intention is to just
> update the section type as long as the name matches, regardless of
> whether there is a section type change.  In both cases, there will be
> cleanup of existing options list.  This is an established behavior and
> should not be changed for backward compatibility considerations.
> 
> Regards,
>                 yousong

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel