ui/ncurses: Don't free item in pmenu_item_setup

Currently pmenu_item_setup may free its item parameter on error.

This makes it non-obvious whether the item is still allocated on exit to
the caller.

Instead, this change removes the talloc_free, and requires that the
caller do this on error. This makes the potential use-after-free in
cui_boot_editor_on_exit obvious, so we fix that too.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index 88bdd0f18a0a35590609075a56fa5fb7e2946f6a..fce1752f04edab8c796464596cc0e8130ac711e9 100644 (file)
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -185,8 +185,11 @@ static void cui_boot_editor_on_exit(struct cui *cui,
                item->data = cod = talloc_zero(item, struct cui_opt_data);
 
                cod->name = talloc_asprintf(cod, "User item %u:", insert_pt);
-               pmenu_item_setup(menu, item, insert_pt,
-                               talloc_strdup(item, cod->name));
+               if (pmenu_item_setup(menu, item, insert_pt,
+                               talloc_strdup(item, cod->name)) == NULL) {
+                       talloc_free(item);
+                       item = NULL;
+               }
 
                /* Re-attach the items array. */
                set_menu_items(menu->ncm, menu->items);
@@ -197,7 +200,8 @@ static void cui_boot_editor_on_exit(struct cui *cui,
 
        cod->bd = talloc_steal(cod, bd);
 
-       set_current_item(item->pmenu->ncm, item->nci);
+       if (item)
+               set_current_item(item->pmenu->ncm, item->nci);
        cui_set_current(cui, &cui->main->scr);
        talloc_free(cui->boot_editor);
        cui->boot_editor = NULL;

diff --git a/ui/ncurses/nc-menu.c b/ui/ncurses/nc-menu.c
index cd7c54e6b67c4dde5fe39d0f613ae701fc6e568e..654c389b4b030114aea346df5e0440be8a5c70ff 100644 (file)
--- a/ui/ncurses/nc-menu.c
+++ b/ui/ncurses/nc-menu.c
@@ -106,10 +106,8 @@ struct pmenu_item *pmenu_item_setup(struct pmenu *menu, struct pmenu_item *i,
        i->pmenu = menu;
        i->nci = new_item(name, NULL);
 
-       if (!i->nci) {
-               talloc_free(i);
+       if (!i->nci)
                return NULL;
-       }
 
        set_item_userptr(i->nci, i);
 

diff --git a/ui/ncurses/nc-menu.h b/ui/ncurses/nc-menu.h
index f5e947d24ea9f59ebe6e5f81361f43786230d043..4c3a43fcca978322d298b220afaf0232b9e98b4e 100644 (file)
--- a/ui/ncurses/nc-menu.h
+++ b/ui/ncurses/nc-menu.h
@@ -75,7 +75,14 @@ static inline struct cui_opt_data *cod_from_item(struct pmenu_item *item)
 static inline struct pmenu_item *pmenu_item_init(struct pmenu *menu,
        unsigned int index, const char *name)
 {
-       return pmenu_item_setup(menu, pmenu_item_alloc(menu), index, name);
+       struct pmenu_item *item = pmenu_item_alloc(menu);
+
+       if (pmenu_item_setup(menu, item, index, name)) {
+               talloc_free(item);
+               item = NULL;
+       }
+
+       return item;
 }
 
 /**