[PATCH v8 5/5] eficonfig: add "Show Signature Database" menu entry

[Masahisa Kojima]

On Fri, 11 Nov 2022 at 21:53, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> [...]
>
> > +/**
> > + * prepare_signature_list_menu() - create the signature list menu entry
> > + *
> > + * @efimenu: pointer to the efimenu structure
> > + * @varname: pointer to the variable name
> > + * @db:              pointer to the variable raw data
> > + * @db_size: variable data size
> > + * @func:    callback of each entry
> > + * Return:   status code
> > + */
> > +static efi_status_t prepare_signature_list_menu(struct efimenu *efi_menu, void *varname,
> > +                                             void *db, efi_uintn_t db_size,
> > +                                             eficonfig_entry_func func)
> > +{
> > +     u32 num = 0;
> > +     efi_uintn_t size;
> > +     struct eficonfig_sig_data *sg;
> > +     struct efi_signature_list *esl;
> > +     struct efi_signature_data *esd;
> > +     efi_status_t ret = EFI_SUCCESS;
> > +
> > +     INIT_LIST_HEAD(&efi_menu->list);
> > +
> > +     esl = db;
> > +     size = db_size;
> > +     while (size > 0) {
> > +             u32 remain;
> > +
> > +             esd = (struct efi_signature_data *)((u8 *)esl +
> > +                                                 (sizeof(struct efi_signature_list) +
> > +                                                 esl->signature_header_size));
> > +             remain = esl->signature_list_size - sizeof(struct efi_signature_list) -
> > +                      esl->signature_header_size;
> > +             for (; remain > 0; remain -= esl->signature_size) {
> > +                     char buf[40];
> > +                     char *title;
> > +
> > +                     if (num >= EFICONFIG_ENTRY_NUM_MAX - 1) {
> > +                             ret = EFI_OUT_OF_RESOURCES;
> > +                             goto out;
> > +                     }
> > +
> > +                     sg = calloc(1, sizeof(struct eficonfig_sig_data));
> > +                     if (!sg) {
> > +                             ret = EFI_OUT_OF_RESOURCES;
> > +                             goto err;
> > +                     }
> > +
> > +                     snprintf(buf, sizeof(buf), "%pUL", &esd->signature_owner);
> > +                     title = calloc(1, (strlen(buf) + 1));
> > +                     if (!title) {
> > +                             free(sg);
> > +                             ret = EFI_OUT_OF_RESOURCES;
> > +                             goto err;
> > +                     }
> > +                     strlcpy(title, buf, strlen(buf) + 1);
> > +
> > +                     sg->esl = esl;
> > +                     sg->esd = esd;
> > +                     sg->varname = varname;
> > +                     ret = eficonfig_append_menu_entry(efi_menu, title, func, sg);
> > +                     if (ret != EFI_SUCCESS) {
> > +                             free(sg);
> > +                             free(title);
> > +                             goto err;
> > +                     }
>
> title and sg are allocated on a for loop.  I assume that those will
> eventually be freed by eficonfig_destroy().  But shouldn't we call that on
> failures as well instead of just freeing the last instance?

Title is freed by eficonfig_destroy(), and entry->data is freed by the
caller because
it depends on the caller's decision whether entry->data must be freed or not.
eficonfig_destroy() also frees "struct efimenu" pointer and it is
allocated by the caller.
As a design same as other eficonfig menu, eficonfig_destroy() is
called by caller,
and entry->data is freed by caller.
The menu creation function(e.g. prepare_signature_list_menu()) frees
the allocated variable
when eficonfig_append_menu_entry() fails because these allocated
variables are not in the
efi_menu->list.

Thanks,
Masahisa Kojima

>
> > +                     esd = (struct efi_signature_data *)((u8 *)esd + esl->signature_size);
> > +                     num++;
> > +             }
> > +
> > +             size -= esl->signature_list_size;
> > +             esl = (struct efi_signature_list *)((u8 *)esl + esl->signature_list_size);
> > +     }
> > +out:
> > +     ret = eficonfig_append_quit_entry(efi_menu);
> > +err:
> > +     return ret;
> > +}
> > +
> > +/**
>
> [...]
>
> Regards
> /Ilias