[OpenWrt-Devel] [PATCH v2] uci: fix options list of section after type change

[Hans Dedecker]

On Fri, May 17, 2019 at 2:30 PM Sven Eckelmann <sven at narfation.org> wrote:
>
> A section can store its name in the same memory region as the section
> (after the actual section object). The object is then reallocated when the
> type is later changed via an uci_set. But the original address of the
> section is (indirectly) stored in the section list, the object and the
> object list (HEAD) of this section.
>
> But only the section list was fixed in commit 4fb6a564b8ee ("clean up
> uci_set") after the realloc finished. Traversing the object list or
> accessing the section pointer caused heap-use-after-free errors.
>
> Reported-by: Charlemagne Lasse <charlemagnelasse at gmail.com>
> Fixes: 4fb6a564b8ee ("clean up uci_set")
> Signed-off-by: Sven Eckelmann <sven at narfation.org>
Patch applied; thx

Hans
> ---
> v2:
>  - drop unnecessary include of stdbool.h in cli.c
>
>  list.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/list.c b/list.c
> index 25aec56..78efbaf 100644
> --- a/list.c
> +++ b/list.c
> @@ -182,6 +182,32 @@ static void uci_fixup_section(struct uci_context *ctx, struct uci_section *s)
>         s->e.name = uci_strdup(ctx, buf);
>  }
>
> +/* fix up option list HEAD pointers and pointer to section in options */
> +static void uci_section_fixup_options(struct uci_section *s, bool no_options)
> +{
> +       struct uci_element *e;
> +
> +       if (no_options) {
> +               /*
> +                * enforce empty list pointer state (s->next == s) when original
> +                * section had no options in the first place
> +                */
> +               uci_list_init(&s->options);
> +               return;
> +       }
> +
> +       /* fix pointers to HEAD at end/beginning of list */
> +       uci_list_fixup(&s->options);
> +
> +       /* fix back pointer to section in options */
> +       uci_foreach_element(&s->options, e) {
> +               struct uci_option *o;
> +
> +               o = uci_to_option(e);
> +               o->section = s;
> +       }
> +}
> +
>  static struct uci_section *
>  uci_alloc_section(struct uci_package *p, const char *type, const char *name)
>  {
> @@ -713,10 +739,15 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
>                 char *s = uci_strdup(ctx, ptr->value);
>
>                 if (ptr->s->type == uci_dataptr(ptr->s)) {
> +                       /* drop the in-section storage of type name */
> +                       bool no_options;
> +
> +                       no_options = uci_list_empty(&ptr->s->options);
>                         ptr->last = NULL;
>                         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_section_fixup_options(ptr->s, no_options);
>                 } else {
>                         free(ptr->s->type);
>                 }
> --
> 2.20.1
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

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